Skip to content
This repository has been archived by the owner on Jun 21, 2024. It is now read-only.

Airtable API integration #22

Merged
merged 5 commits into from
Dec 13, 2021
Merged

Airtable API integration #22

merged 5 commits into from
Dec 13, 2021

Conversation

chrisdevereux
Copy link
Contributor

Description

Adds an Airtable datasource. Adapts the existing RestDatasource to Airtable's API conventions and some conveniences for getting at a base API via its id and table name.

Motivation and Context

Closes #18

How Can It Be Tested?

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Documentation change

Checklist:

  • I have documented all new methods using google docstring format.
  • I have added type annotations to any public API methods.
  • I have updated any relevant high-level documentation.
  • I have added a usage example to the example app if relevant.
  • I have written tests covering all new changes.

@janbaykara
Copy link
Member

Exciting! I'll have a look now.

Copy link
Member

@janbaykara janbaykara left a comment

Choose a reason for hiding this comment

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

Looks good, simple. It looks like there's no official Airtable python package, but there are a few unofficial ones. Was there a reason you didn't lean on one of those?

resource_type=MyResource,
api_key=settings.EXAMPLE_AIRTABLE_API_KEY,
base_id=settings.EXAMPLE_AIRTABLE_BASE,
table_name="Table 1",
Copy link
Member

Choose a reason for hiding this comment

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

table_name could also go on the env as settings.EXAMPLE_AIRTABLE_TABLE_NAME? Appreciate that it is the default table name, but it can be changed by the user of course.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Main reason is that it introduces another environmental variable that needs to be shifted between different environments that this needs to run in (local dev machines, ci etc). Secondary reason – we might plausably introduce an integration test for multiple tables (if we wanted to test relationships, for example) so making it explicit in the code which table is being referenced in a test is useful.

@chrisdevereux
Copy link
Contributor Author

chrisdevereux commented Dec 13, 2021

Looks good, simple. It looks like there's no official Airtable python package, but there are a few unofficial ones. Was there a reason you didn't lean on one of those?

Didn't really seem like there's any point introducing a third-party dependency given that it'd just be a very slim wrapper around the REST api anyway (and Airtable has quite a simple API with good documentation). An unecessary additional moving part. Most of the general 'rest client' stuff we have in hand via our own RestDatasource anyway.

@chrisdevereux chrisdevereux merged commit e186603 into main Dec 13, 2021
@chrisdevereux chrisdevereux deleted the airtable branch December 13, 2021 16:38
@chrisdevereux chrisdevereux restored the airtable branch February 10, 2022 22:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Airtable integration
2 participants