add the ability to upgrade CDK for java connectors #34343

stephane-airbyte · 2024-01-18T00:49:40Z

We already had the ability to update the CDK version for python connectors. I'm just adding the same thing for java connectors.

It required some minor refactoring. I haven't added any tests yet, but I tested it locally

vercel · 2024-01-18T00:49:46Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
airbyte-docs	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jan 25, 2024 3:58pm

stephane-airbyte · 2024-01-18T00:49:53Z

master
- add the ability to upgrade CDK for java connectors #34343 👈
  - add connector commands to add entries to changelog #34371
  - add connectors command to add entries to the changelog #34370

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @stephane-airbyte and the rest of your teammates on Graphite

github-actions · 2024-01-18T00:50:03Z

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

PR name follows PR naming conventions
Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
Secrets in the connector's spec are annotated with airbyte_secret
All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

Check for hidden checklists in your PR description
Toggle the github label checklist-action-run on/off to re-run the checklist CI.

stephane-airbyte · 2024-01-18T00:51:39Z

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/upgrade_cdk/commands.py

 @click.command(cls=DaggerPipelineCommand, short_help="Upgrade CDK version")
-@click.argument("target-cdk-version", type=str, default=latest_cdk_version)
+@click.argument("target-cdk-version", type=str, default="latest")


now, "latest" has different meaning for python vs java CDK. Therefore, I'm just passing the "latest" string and will resolve it later

stephane-airbyte · 2024-01-18T00:52:14Z

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/upgrade_cdk/pipeline.py

@@ -55,17 +60,77 @@ async def _run(self) -> StepResult:
                exc_info=e,
            )

-    def update_cdk_version(self, og_setup_py_content: str) -> str:
+    async def upgrade_cdk_version_for_java_connector(self, og_connector_dir) -> StepResult:


this is new, and handling java only

stephane-airbyte · 2024-01-18T00:52:34Z

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/upgrade_cdk/pipeline.py

+            raise ValueError("Could not find airbyte-cdk dependency in build.gradle")
+
+        if self.new_version == "latest":
+            version_file_content = await self.context.get_repo_file(


java "latest" transaltion

stephane-airbyte · 2024-01-18T00:53:21Z

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/upgrade_cdk/pipeline.py

            raise ValueError("Could not find airbyte-cdk dependency in setup.py")

+        if self.new_version == "latest":
+            """


this is lifted from the original latest_cdk_version that was in commands.py

stephane-airbyte · 2024-01-18T01:30:46Z

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/upgrade_cdk/pipeline.py

        airbyte_cdk_dependency = re.search(
-            r"airbyte-cdk(?P<extra>\[[a-zA-Z0-9-]*\])?(?P<version>[<>=!~]+[0-9]*\.[0-9]*\.[0-9]*)?", og_setup_py_content
+            r"airbyte-cdk(?P<extra>\[[a-zA-Z0-9-]*\])?(?P<version>[<>=!~]+[0-9]*(?:\.[0-9]*)?(?:\.[0-9]*)?)?", setup_py_content


the old regexp would not match airbyte-cdk~=0.11, or airbyte-cdk~=1

alafanechere

I really appreciate the effort to keep a language agnostic command, thanks!

I added minor suggestions, approving to not be blocking, but please consider them.

alafanechere · 2024-01-19T08:17:50Z

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/upgrade_cdk/pipeline.py

-            updated_setup_py = self.update_cdk_version(setup_py_content)
-            updated_connector_dir = og_connector_dir.with_new_file("setup.py", updated_setup_py)
+            og_connector_dir = await context.get_connector_dir()
+            if self.context.connector.language == ConnectorLanguage.PYTHON or self.context.connector.language == ConnectorLanguage.LOW_CODE:


Suggestion:
if self.context.connector.language in [ConnectorLanguage.PYTHON, ConnectorLanguage.LOW_CODE]

alafanechere · 2024-01-19T08:18:12Z

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/upgrade_cdk/pipeline.py

+            og_connector_dir = await context.get_connector_dir()
+            if self.context.connector.language == ConnectorLanguage.PYTHON or self.context.connector.language == ConnectorLanguage.LOW_CODE:
+                updated_connector_dir = await self.upgrade_cdk_version_for_python_connector(og_connector_dir)
+            elif self.context.connector.language == ConnectorLanguage.JAVA:


To check if something matches an enum value please use is instead of ==.
It's equivalent but I think its recommended by our linter and I find it more pythonic.
elif self.context.connector.language is ConnectorLanguage.JAVA

alafanechere · 2024-01-19T08:19:29Z

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/upgrade_cdk/pipeline.py

+                return StepResult(
+                    self,
+                    StepStatus.FAILURE,
+                    f"Can't upgrade the CDK for connector {self.context.connector.technical_name} of written in {self.context.connector.language}",


Suggested change

f"Can't upgrade the CDK for connector {self.context.connector.technical_name} of written in {self.context.connector.language}",

f"No CDK found connector {self.context.connector.technical_name} written in {self.context.connector.language}",

alafanechere · 2024-01-19T08:31:02Z

airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/upgrade_cdk/pipeline.py

+            cdk_pypi_url = "https://pypi.org/pypi/airbyte-cdk/json"
+            response = requests.get(cdk_pypi_url)
+            response.raise_for_status()
+            package_info = response.json()
+            new_version = package_info["info"]["version"]


Can we keep this logic in a separate function? Maybe in a new cdk module under pipelines/helpers/connectors/cdk.py

I'd also find it useful to store your logic to get the latest Java cdk version in there.

alafanechere

Requesting a review from @flash1293 as he's the original contributor to this command and uses it in the python cdk release flox.

flash1293

I didn't get to add that to the CDK release pipeline yet, so changes are inconsequential. Makes total sense to extend this for the Java CDK as well!

…CDK_for_java_connectors

stephane-airbyte · 2024-01-25T16:10:50Z

Merge activity

Jan 25, 11:10 AM: @stephane-airbyte merged this pull request with Graphite.

We already had the ability to update the CDK version for python connectors. I'm just adding the same thing for java connectors. It required some minor refactoring. I haven't added any tests yet, but I tested it locally

octavia-squidington-iii added area/connectors Connector related issues connectors/source/postgres labels Jan 18, 2024

stephane-airbyte commented Jan 18, 2024

View reviewed changes

stephane-airbyte force-pushed the stephane/01-17-add_the_ability_to_upgrade_CDK_for_java_connectors branch from ec13a23 to 6ad2ade Compare January 18, 2024 00:54

octavia-squidington-iii removed the area/connectors Connector related issues label Jan 18, 2024

stephane-airbyte force-pushed the stephane/01-17-add_the_ability_to_upgrade_CDK_for_java_connectors branch 2 times, most recently from 2f86adf to 6eb4d64 Compare January 18, 2024 01:29

stephane-airbyte commented Jan 18, 2024

View reviewed changes

stephane-airbyte requested a review from alafanechere January 18, 2024 16:12

stephane-airbyte force-pushed the stephane/01-17-add_the_ability_to_upgrade_CDK_for_java_connectors branch 4 times, most recently from 4ac0432 to 5873c2c Compare January 18, 2024 19:41

stephane-airbyte marked this pull request as ready for review January 18, 2024 19:43

stephane-airbyte requested a review from a team January 18, 2024 19:43

vercel bot deployed to Preview January 18, 2024 19:44 View deployment

stephane-airbyte force-pushed the stephane/01-17-add_the_ability_to_upgrade_CDK_for_java_connectors branch from 73aca55 to 7e4a67e Compare January 18, 2024 20:51

This was referenced Jan 19, 2024

add connectors command to add entries to the changelog #34370

Closed

add connector commands to add entries to changelog #34371

Closed

alafanechere approved these changes Jan 19, 2024

View reviewed changes

alafanechere requested a review from flash1293 January 19, 2024 11:30

alafanechere reviewed Jan 19, 2024

View reviewed changes

flash1293 approved these changes Jan 19, 2024

View reviewed changes

stephane-airbyte force-pushed the stephane/01-17-add_the_ability_to_upgrade_CDK_for_java_connectors branch from 7e4a67e to 0df2723 Compare January 25, 2024 04:42

stephane-airbyte force-pushed the stephane/01-17-add_the_ability_to_upgrade_CDK_for_java_connectors branch from 0df2723 to 64e5b0a Compare January 25, 2024 04:45

add the ability to upgrade CDK for java connectors

0345e5f

alafanechere force-pushed the stephane/01-17-add_the_ability_to_upgrade_CDK_for_java_connectors branch from 64e5b0a to 0345e5f Compare January 25, 2024 11:01

Merge branch 'master' into stephane/01-17-add_the_ability_to_upgrade_…

368a9ff

…CDK_for_java_connectors

vercel bot deployed to Preview January 25, 2024 15:58 View deployment

stephane-airbyte merged commit 6c4a75d into master Jan 25, 2024
24 checks passed

stephane-airbyte deleted the stephane/01-17-add_the_ability_to_upgrade_CDK_for_java_connectors branch January 25, 2024 16:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add the ability to upgrade CDK for java connectors #34343

add the ability to upgrade CDK for java connectors #34343

stephane-airbyte commented Jan 18, 2024 •

edited

Loading

vercel bot commented Jan 18, 2024 •

edited

Loading

stephane-airbyte commented Jan 18, 2024 •

edited

Loading

github-actions bot commented Jan 18, 2024

stephane-airbyte Jan 18, 2024

stephane-airbyte Jan 18, 2024

stephane-airbyte Jan 18, 2024

stephane-airbyte Jan 18, 2024

stephane-airbyte Jan 18, 2024

alafanechere left a comment •

edited

Loading

alafanechere Jan 19, 2024

alafanechere Jan 19, 2024

alafanechere Jan 19, 2024

alafanechere Jan 19, 2024

alafanechere left a comment

flash1293 left a comment

stephane-airbyte commented Jan 25, 2024

	f"Can't upgrade the CDK for connector {self.context.connector.technical_name} of written in {self.context.connector.language}",
	f"No CDK found connector {self.context.connector.technical_name} written in {self.context.connector.language}",

add the ability to upgrade CDK for java connectors #34343

add the ability to upgrade CDK for java connectors #34343

Conversation

stephane-airbyte commented Jan 18, 2024 • edited Loading

vercel bot commented Jan 18, 2024 • edited Loading

stephane-airbyte commented Jan 18, 2024 • edited Loading

github-actions bot commented Jan 18, 2024

Before Merging a Connector Pull Request

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alafanechere left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alafanechere left a comment

Choose a reason for hiding this comment

flash1293 left a comment

Choose a reason for hiding this comment

stephane-airbyte commented Jan 25, 2024

Merge activity

stephane-airbyte commented Jan 18, 2024 •

edited

Loading

vercel bot commented Jan 18, 2024 •

edited

Loading

stephane-airbyte commented Jan 18, 2024 •

edited

Loading

alafanechere left a comment •

edited

Loading