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

feat: add version with dataclasses #91

Merged
merged 7 commits into from
Aug 23, 2024
Merged

Conversation

artem1205
Copy link
Contributor

@artem1205 artem1205 commented Aug 14, 2024

What

Follow up airbytehq/airbyte#44026
Generate another version of the python protocol, by generating dataclasses instead of pydantic.v2 BaseModel.
Resolving https://github.com/airbytehq/airbyte-internal-issues/issues/8945
Reason: increase python CDK performance in airbytehq/airbyte#44026

Note

Dataclass version will have different package name : airbyte_protocol_dataclasses, so we can import both pydantic and dataclasses versions at the same time

How

Use datamodel-codegen to create the models from jsonschema, with flag --output-model-type dataclasses.dataclass

@artem1205 artem1205 self-assigned this Aug 14, 2024
@artem1205 artem1205 changed the title Airbyte Protocol: add dataclasses feat: add version with dataclasses Aug 14, 2024
@artem1205 artem1205 requested a review from erohmensing August 15, 2024 08:19
@artem1205 artem1205 requested a review from girarda August 22, 2024 07:56
Copy link
Contributor

@girarda girarda left a comment

Choose a reason for hiding this comment

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

change looks good but airbyte_protocol_dataclasses deserves a readme explaining that it's just an alternate implementation of the protocol in python using dataclasses because they have less performance overhead than pydantic

"Topic :: Scientific/Engineering",
"Topic :: Software Development :: Libraries :: Python Modules",
"License :: OSI Approved :: MIT License",
# Python Version Support
Copy link
Contributor

Choose a reason for hiding this comment

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

3.10 too, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added up to 3.11

--input "$ROOT_DIR/$YAML_DIR/$filename_wo_ext.yaml" \
--output "$ROOT_DIR/$OUTPUT_DIR/$filename_wo_ext.py" \
--output-model-type dataclasses.dataclass \
--target-python-version 3.10 \
Copy link
Contributor

Choose a reason for hiding this comment

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

should we target 3.8 since that's the minimum version?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Leave 3.8 support sincee 3.8 and 3.10 version are identical.

@artem1205 artem1205 merged commit 5aa44db into main Aug 23, 2024
6 checks passed
@artem1205 artem1205 deleted the artem1205/add-dataclasses-option branch August 23, 2024 16:44
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