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

connectors-ci: handle strict encrypt connectors #25864

Merged

Conversation

alafanechere
Copy link
Contributor

What

Closes #25846

The strict encrypt connectors were not testable with the CLI because we used the OSS registry to get the list of available connectors.

How

  • Create the list of available connectors by searching all the folder having a metadata.yaml file
  • Tweak the gradle command run to include non strict encrypt connector project when building / testing strict encrypt connectors.

@alafanechere alafanechere marked this pull request as ready for review May 6, 2023 16:39
Copy link
Contributor

@bnchrch bnchrch left a comment

Choose a reason for hiding this comment

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

Couple of nits, otherwise lgtm!

@@ -430,10 +430,16 @@ def build_include(self) -> List[str]:
Returns:
List[str]: List of directories or files to be mounted to the container to run a Java connector Gradle task.
"""
additional_inclusion = []
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Why an array and not a single value?

@@ -430,10 +430,16 @@ def build_include(self) -> List[str]:
Returns:
List[str]: List of directories or files to be mounted to the container to run a Java connector Gradle task.
"""
additional_inclusion = []
if self.context.connector.technical_name.endswith("-strict-encrypt"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Idea: it would be great if we had a more explicit mapping

related_connectors = {
  "source-file-secure": ["source-file"],
  "source-postgres-strict-encrypt": ["source-postgres"]
  # ...
,}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd prefer to not hardcode and maintain the list of strict-encrypted connectors in the pipeline.
I created a dedicated function that will hopefully provide a bit more readability: dcb6bcc

@alafanechere alafanechere enabled auto-merge (squash) May 9, 2023 07:59
@alafanechere alafanechere disabled auto-merge May 9, 2023 08:26
@alafanechere alafanechere merged commit 7cc2218 into master May 9, 2023
14 checks passed
@alafanechere alafanechere deleted the augustin/connectors-ci/handle-strict-encrypt-connectors branch May 9, 2023 08:26
marcosmarxm pushed a commit to natalia-miinto/airbyte that referenced this pull request Jun 8, 2023
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.

connectors-ci: strict encrypt connector should be testable and publishable with the CLI
2 participants