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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

airbyte-ci: re-enable connector dependency upload on publish #36962

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented Apr 10, 2024

Change the uploaded file schema. dependencies is now an array of objects with package_name and version keys. This should make it easier for the data team to ingest this data.

This format has been validated by the data team


Ellipsis 馃殌 This PR description was created by Ellipsis for commit 8947e6d.

Summary:

This PR re-enables the upload of connector dependencies on publish with a new schema for dependencies and updates the version in pyproject.toml.

Key points:

  • Re-enabled connector dependency upload on publish.
  • Changed dependencies to an array of objects with package_name and version keys in ConnectorDependenciesMetadata class in /airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/publish/pipeline.py.
  • Updated version in pyproject.toml.

Generated with 鉂わ笍 by ellipsis.dev

Copy link

vercel bot commented Apr 10, 2024

The latest updates on your projects. Learn more about Vercel for Git 鈫楋笌

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs 猬滐笍 Ignored (Inspect) Visit Preview Apr 11, 2024 2:28pm

Copy link
Contributor Author

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

Join @alafanechere and the rest of your teammates on Graphite Graphite

@alafanechere alafanechere marked this pull request as ready for review April 11, 2024 14:26
@alafanechere alafanechere requested a review from a team as a code owner April 11, 2024 14:26
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

馃憤 Looks good to me!

  • Reviewed the entire pull request up to a02c0f8
  • Looked at 67 lines of code in 3 files
  • Took 55 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 50%.
1. airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/publish/pipeline.py:87:
  • Assessed confidence : 0%
  • Comment:
    The change in the schema of the dependencies from a dictionary to a list of dictionaries is correctly done. The re-enabling of the upload dependencies step is also correctly done.
  • Reasoning:
    The changes in the PR look good. The change in the schema of the dependencies from a dictionary to a list of dictionaries should not cause any issues. The re-enabling of the upload dependencies step is also correctly done. The version bump in the pyproject.toml file is also correctly done.

Workflow ID: wflow_zi04PJhRVD1JLUPe


Not what you expected? You can customize the content of the reviews using rules. Learn more here.

@alafanechere alafanechere force-pushed the augustin/04-10-airbyte-ci_re-enable_connector_dependency_upload_on_publish branch from a02c0f8 to 8947e6d Compare April 11, 2024 14:27
Copy link

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

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

馃憤 Looks good to me!

  • Performed an incremental review on 8947e6d
  • Looked at 75 lines of code in 3 files
  • Took 2 minutes and 4 seconds to review
More info
  • Skipped 0 files when reviewing.
  • Skipped posting 1 additional comments because they didn't meet confidence threshold of 50%.
1. /airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/connectors/publish/pipeline.py:87:
  • Assessed confidence : 0%
  • Comment:
    The changes in the PR seem to be in line with the PR description. The dependencies have been changed to an array of objects with package_name and version keys. The version in pyproject.toml has been updated. The connector dependency upload on publish has been re-enabled. There don't seem to be any logical bugs, performance bugs, or security bugs. The code seems to follow best practices.
  • Reasoning:
    The changes in the PR seem to be in line with the PR description. The dependencies have been changed to an array of objects with package_name and version keys. The version in pyproject.toml has been updated. The connector dependency upload on publish has been re-enabled. There don't seem to be any logical bugs, performance bugs, or security bugs. The code seems to follow best practices.

Workflow ID: wflow_nwROAMLevfNXctpq


Not what you expected? You can customize the content of the reviews using rules. Learn more here.

@alafanechere
Copy link
Contributor Author

/approve-and-merge reason="re-enabling an existing feature, new schema has been validated by the data team"

@octavia-approvington
Copy link
Contributor

A crack team of mammals has made a decision.
imagine a seal of approval

@octavia-approvington octavia-approvington merged commit 0571d18 into master Apr 11, 2024
33 checks passed
@octavia-approvington octavia-approvington deleted the augustin/04-10-airbyte-ci_re-enable_connector_dependency_upload_on_publish branch April 11, 2024 14:58
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.

None yet

2 participants