-
Notifications
You must be signed in to change notification settings - Fork 32
fix: (PoC) (do not merge) (CDK) (Manifest) - Migrate manifest fields #463
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
Conversation
…ll migrations available
|
somewhat minor comment: should this code live outside of the |
|
another high-level thing. Before you open this PR for review, I wonder if it makes sense to break down the changes into several related PRs since there is a lot happening here. What about something like this? PR1: introduce the migration framework |
| ) | ||
|
|
||
|
|
||
| class V_6_45_2_ManifestMigration_HttpRequesterUrlBaseToUrl(ManifestMigration): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙇
What
Resolves:
How
Manifest Migrations:
Introduced a framework to handle manifest migrations in the
Airbyte CDKto apply transformations on given manifest.Added migration logic to convert
url_baseandpathtourl.http_requester_url_base_to_url_v6_45_2__0.pyhttp_requester_path_to_url_v6_45_2__1.pyCreated migration files with clear
versioningandorderhandling.Registered migration classes dynamically and applied them in order.
Updated Component Models:
HttpRequesterto support the newurlfield and prioritize it.url_baseandpathfields.Deprecation of url_base and path fields:
HttpRequester.urlfield to replace the deprecatedurl_baseandpathfields.url_baseandpathasdeprecatedwith deprecation messages.Handling Deprecation Warnings:
deprecation_warningsmethod to theDeclarativeSourceandManifestDeclarativeSourceclass to collect and process deprecation notices._categorise_groups()to handle deprecation warnings alongside message groups.BaseModelWithDeprecations.airbyte_cdk/sources/declarative/models/base_model_with_deprecations.pymodule is added to cover thedeprecationwarnings for the fields marked asdeprecated. This class is static (not autogenerated) and re-used when thedeclarative_component_schema.pyis being generated. In this way, there is no need to set additional validations on top of the existing component implementations.Unit Tests and Documentation:
unit_tests/sources/declarative/migrations/test_manifest_migration.pyREADME.mdin themanifest/migrationsdirectory to document the migration framework.User Impact
migrate_manifest: bool = Falseby default, to not to have any regressions, before we're ready to use it within the UI (/resolveshould be havingmigrate: boolflag to set the migration toTrue)Related PR parts:
Manifest Normalizer- fix: (CDK) (Manifest) - AddManifest Normalizationmodule (reduce commonalities + handle schema $refs) #447Deprecation Warning examples
The deprecation rules applied to the
declarative_component_schema.yamlare automatically picked up and propagated to thedeclarative_component_schema.py(auto-gen models), see the example bellow.Warning example
When
usingthe deprecated field orcreatingan instance of the class with the deprecated field.Python default behavior:
If multiple fields are marked as
deprecatedand used - more than 1 deprecation message is shown.Connector Builder behavior: