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

CDK: Add base pydantic model for connector config and schemas #8485

Merged
merged 11 commits into from
Dec 7, 2021

Conversation

keu
Copy link
Contributor

@keu keu commented Dec 3, 2021

What

When we use pydantic model instead of spec.json we often duplicate the same piece of logic every time. It would be nice to have some common base model provided by CDK that will:

  • replace $ref with a definition
  • replace anyOf with oneOf
  • extra postprocessing
  • render Optional fields as nullable
  • enforce all fields to be Optional (configurable)

How

Define config.BaseConfig and sources.utils.schema_models.BaseSchemaModel classes that can be used instead of pydantic.BaseModel

Recommended reading order

  1. x.java
  2. y.python

🚨 User Impact 🚨

Doesn't change existing behavior. Existing connectors need an update to start using the piece of code added in this PR.

Pre-merge Checklist

Expand the relevant checklist and delete the others.

@CLAassistant
Copy link

CLAassistant commented Dec 3, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ keu
❌ eugene-kulak
You have signed the CLA already but the status is still pending? Let us recheck it.

@github-actions github-actions bot added the CDK Connector Development Kit label Dec 3, 2021
@keu keu temporarily deployed to more-secrets December 3, 2021 13:28 Inactive
@keu keu temporarily deployed to more-secrets December 6, 2021 14:26 Inactive
@keu keu force-pushed the keu/cdk-base-pydantic-models branch from f2cfb49 to 0b7674e Compare December 6, 2021 14:27
@keu keu temporarily deployed to more-secrets December 6, 2021 14:28 Inactive
@keu keu temporarily deployed to more-secrets December 6, 2021 14:46 Inactive
@keu keu temporarily deployed to more-secrets December 6, 2021 14:52 Inactive
@keu keu temporarily deployed to more-secrets December 6, 2021 19:47 Inactive
@keu keu temporarily deployed to more-secrets December 7, 2021 07:49 Inactive
@keu keu temporarily deployed to more-secrets December 7, 2021 09:57 Inactive
@keu keu temporarily deployed to more-secrets December 7, 2021 10:05 Inactive
@keu keu changed the title CDK: Add base pydantic model for connector config CDK: Add base pydantic model for connector config and schemas Dec 7, 2021
@keu keu temporarily deployed to more-secrets December 7, 2021 10:27 Inactive
@@ -186,6 +185,7 @@ def _read_incremental(
for record_counter, record_data in enumerate(records, start=1):
yield self._as_airbyte_record(stream_name, record_data)
stream_state = stream_instance.get_updated_state(stream_state, record_data)
checkpoint_interval = stream_instance.state_checkpoint_interval
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this fixes issue with dynamic behaviour of state_checkpoint_interval (may change after read call)

Copy link
Contributor

@eliziario eliziario left a comment

Choose a reason for hiding this comment

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

Really nice.

@keu keu added this to the ConnCore Dec 8, 2021 milestone Dec 7, 2021
@keu keu temporarily deployed to more-secrets December 7, 2021 22:53 Inactive
@keu
Copy link
Contributor Author

keu commented Dec 7, 2021

/publish-cdk dry-run=false

🕑 https://github.com/airbytehq/airbyte/actions/runs/1551620268
https://github.com/airbytehq/airbyte/actions/runs/1551620268

@keu keu temporarily deployed to more-secrets December 7, 2021 23:02 Inactive
@keu keu merged commit aa67604 into master Dec 7, 2021
@keu keu deleted the keu/cdk-base-pydantic-models branch December 7, 2021 23:15
@sherifnada sherifnada removed this from the ConnCore Dec 8, 2021 milestone Dec 8, 2021
@keu keu added this to the Connectors Dec 10 2021 milestone Dec 9, 2021
schlattk pushed a commit to schlattk/airbyte that referenced this pull request Jan 4, 2022
…ehq#8485)

* add base spec model

* fix usage of state_checkpoint_interval in case it is dynamic

* add schema base models, fix spelling, signatures and polishing

Co-authored-by: Eugene Kulak <kulak.eugene@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CDK Connector Development Kit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants