{{ message }}
Repository navigation
Stop logging a warning per webhook delivery - #83
Draft
aiolibsbot wants to merge 2 commits into
Draft
aiolibsbot wants to merge 2 commits into
aiolibsbot wants to merge 2 commits into
Conversation
`octomachinery` maps the `/app/installations/{id}` API response onto its
`GitHubAppInstallation` model through `dict_to_kwargs_cb()`, which drops the
payload keys the model does not declare and logs `Excessive arguments passed to
callback` when it does so. GitHub has since added `client_id` and
`contact_email` to that response, so the model no longer covers it and every
webhook delivery — each one looks the installation up — emits that warning.
Widen the model with the two fields instead of silencing the logger: the
payload then maps onto it exactly, which also keeps the values instead of
throwing them away. Both fields default to `None`, so an older payload that
omits them still deserializes.
This is a stopgap in the app because the pinned `octomachinery` is what lags
behind the API; the patch becomes redundant once an upstream release declares
these fields.
Refs sanitizers#82
Member
Author
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Every webhook delivery logs
WARNING:octomachinery.utils.asynctools:Excessive arguments passed to callback <class 'octomachinery.github.models.GitHubAppInstallation'>because GitHub added
client_idandcontact_emailto the/app/installations/{id}response and the pinnedoctomachinery's model doesnot declare them —
dict_to_kwargs_cb()drops the unknown keys and warns. Thiswidens the model with those two fields, so the payload maps onto it exactly and
the values are kept instead of discarded.
Stopgap in the app because
octomachineryis what lags behind the API; itbecomes redundant once an upstream release declares these fields.
Refs #82
Changes
patchback/octomachinery_compat.py: an installation model subclass addingclient_idandcontact_email(both defaulting toNone, so pre-client_idpayloads still deserialize), plus
patch_github_app_installation_model()rebindingoctomachinery.github.api.app_client.GitHubAppInstallationModel.patchback/__main__.py: applies the patch at startup.tests/test_octomachinery_compat.py: covers the warning and the mapping.Test plan
python -m pytest tests/test_octomachinery_compat.py— 3 passed. The testsassert that today's GitHub payload maps onto the patched model with no
WARNINGrecords and both values retained, that a payload without the newkeys still works, and that the unpatched upstream model is what warns.
patchback.__main__withoctomachineryinstalled and confirmedapp_client.GitHubAppInstallationModelis the widened model.no test workflow on
masterandpytestis not inrequirements.txt, so CIdoes not execute it yet.
Quality Report
Changes: 3 files changed, 165 insertions(+)
Code scan: clean
Tests: skipped
Branch hygiene: clean
Generated by Kōan