Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Webhooks support #291

Merged
merged 11 commits into from
Aug 11, 2023
Merged

Webhooks support #291

merged 11 commits into from
Aug 11, 2023

Conversation

mesozoic
Copy link
Collaborator

@mesozoic mesozoic commented Aug 6, 2023

This branch introduces models representing the webhooks API along with methods for validating notifications and for retrieving new webhook payloads. It adds the following methods and classes to the public API:

  • Base.add_webhook(url, spec)
  • Base.webhooks()
  • Base.webhook(id)
  • models.WebhookNotification
  • models.WebhookNotification.from_request(body, header, secret)
  • models.Webhook
  • models.Webhook.{enable,disable}_notifications()
  • models.Webhook.extend_expiration()
  • models.Webhook.payloads(cursor=1)
  • models.WebhookPayload

The major design decision here is to not return plain old dict data structures from the API, and instead to parse them into nested pydantic data models with type annotations, using extra = "ignore" so that it doesn't crash if Airtable adds more fields in the future.

The advantage of this approach is a cleaner API and more assistance for developers who use type-aware IDEs; the disadvantage is that any new fields which Airtable adds to their API in the future will require a patch release to fully support. I've worked around this by adding a private attribute called _raw to every model which will be the API payload which generated the model. In a pinch, a developer could access that directly if it contains anything our models don't recognize.

Tagging a few folks for input:

@codecov
Copy link

codecov bot commented Aug 6, 2023

Codecov Report

Merging #291 (239f61f) into main (31242b2) will increase coverage by 0.52%.
The diff coverage is 98.19%.

@@            Coverage Diff             @@
##             main     #291      +/-   ##
==========================================
+ Coverage   95.51%   96.04%   +0.52%     
==========================================
  Files          20       21       +1     
  Lines         937     1138     +201     
==========================================
+ Hits          895     1093     +198     
- Misses         42       45       +3     
Files Changed Coverage Δ
pyairtable/api/params.py 100.00% <ø> (ø)
pyairtable/orm/model.py 97.56% <ø> (ø)
pyairtable/api/base.py 87.80% <80.00%> (-7.65%) ⬇️
pyairtable/api/api.py 97.56% <100.00%> (+0.06%) ⬆️
pyairtable/models/__init__.py 100.00% <100.00%> (ø)
pyairtable/models/_base.py 100.00% <100.00%> (+2.22%) ⬆️
pyairtable/models/comment.py 100.00% <100.00%> (ø)
pyairtable/models/webhook.py 100.00% <100.00%> (ø)

@marks
Copy link
Contributor

marks commented Aug 7, 2023

Hi @mesozoic - thanks for doing this! I took a quick skim and it looks good to me.

No imminent changes and I think it's smart to ensure additional fields do not break types/etc as the deprecation guidelines note:

Note that adding new data in a forwards-compatible way is not considered a breaking change (e.g. adding new keys and values to an object returned from an API endpoint).

@mesozoic mesozoic merged commit 6d8f7b1 into gtalarico:main Aug 11, 2023
7 checks passed
@mesozoic mesozoic deleted the webhooks branch August 11, 2023 23:47
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.

2 participants