Skip to content

Add Webhook Deliveries#2508

Merged
EnricoMi merged 6 commits intoPyGithub:masterfrom
jmgreg31:feature/jmgreg-webhook-deliveries
Jun 20, 2023
Merged

Add Webhook Deliveries#2508
EnricoMi merged 6 commits intoPyGithub:masterfrom
jmgreg31:feature/jmgreg-webhook-deliveries

Conversation

@jmgreg31
Copy link
Contributor

Webhook Deliveries

Add support for Organization and Repository Webhook Deliveries

Summary of Changes

  • Added two methods to MainClass, Organization, and Repository to support get_hook_delivery and get_hook_deliveries
  • Created a HookDelivery model where github.HookDelivery.HookDelivery is a subclass of github.HookDelivery.HookDeliverySummary for DRY implementation
  • Added test cases to Organization and Repository for validation

Resolves #2507

@codecov-commenter
Copy link

codecov-commenter commented Apr 23, 2023

Codecov Report

Patch coverage: 97.54% and project coverage change: -0.01 ⚠️

Comparison is base (804c310) 98.33% compared to head (d00e9ed) 98.32%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2508      +/-   ##
==========================================
- Coverage   98.33%   98.32%   -0.01%     
==========================================
  Files         130      131       +1     
  Lines       12978    13140     +162     
==========================================
+ Hits        12762    12920     +158     
- Misses        216      220       +4     
Impacted Files Coverage Δ
github/HookDelivery.py 97.01% <97.01%> (ø)
github/GithubObject.py 96.42% <100.00%> (ø)
github/MainClass.py 99.29% <100.00%> (+0.02%) ⬆️
github/Organization.py 98.83% <100.00%> (+0.01%) ⬆️
github/Repository.py 97.15% <100.00%> (+0.01%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@trim21
Copy link
Contributor

trim21 commented May 13, 2023

Can we write inline type annotation instead of seprated pyi for new file?

#2481

@jmgreg31 jmgreg31 force-pushed the feature/jmgreg-webhook-deliveries branch 5 times, most recently from cab4757 to 934681e Compare May 15, 2023 14:04
@jmgreg31
Copy link
Contributor Author

jmgreg31 commented May 15, 2023

Can we write inline type annotation instead of seprated pyi for new file?

#2481

@trim21 This ended up being a bit more difficult trying to bridge the gap from existing .pyi structure. I believe I was able to get this implemented correctly, but had to make some updates to other .pyi file dependancies to pass linting across all python versions. Apologies if folks were getting emailed about all the CI builds while testing, I eventually realized I was getting the same mypy linting errors locally and was able to continue validation without triggering the CI

Copy link
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Excellent pull request, can we extend the tests please? I think the replay data are already sufficient, just more assertions.

@jmgreg31
Copy link
Contributor Author

@EnricoMi Added assertions for all attributes in Organization and Repository as well as adding the same replay/test workflow to Github_.py to test MainClass directly

@jmgreg31 jmgreg31 force-pushed the feature/jmgreg-webhook-deliveries branch from 08c0f35 to acb8a62 Compare May 26, 2023 03:55
@jmgreg31 jmgreg31 requested a review from EnricoMi May 31, 2023 02:46
@jmgreg31 jmgreg31 force-pushed the feature/jmgreg-webhook-deliveries branch 2 times, most recently from 5765c0b to a2428df Compare June 10, 2023 10:43
@EnricoMi EnricoMi force-pushed the feature/jmgreg-webhook-deliveries branch from a2428df to 16ffa39 Compare June 20, 2023 05:48
@EnricoMi EnricoMi changed the title feature: Webhook Deliveries Add Webhook Deliveries Jun 20, 2023
@EnricoMi
Copy link
Collaborator

@jmgreg31 sorry for the delay. We have reworked some related code, which broke your pull request. I have resolved the issues and changed delivered_at into a datetime attribute.

Copy link
Collaborator

@EnricoMi EnricoMi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!



class PaginatedList(PaginatedListBase):
class PaginatedList(PaginatedListBase, Generic[T]):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this generic is not actually used in this pr?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

resolved

@EnricoMi EnricoMi merged commit 517ad33 into PyGithub:master Jun 20, 2023
@jmgreg31
Copy link
Contributor Author

jmgreg31 commented Jun 20, 2023

Thanks for the reviews and merge! @EnricoMi @trim21

@jmgreg31 jmgreg31 deleted the feature/jmgreg-webhook-deliveries branch June 20, 2023 12:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ENHANCEMENT: Support for Webhook Deliveries

4 participants