-
Notifications
You must be signed in to change notification settings - Fork 4.5k
[airbyte-ci] new commands for migration to base image #30520
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
[airbyte-ci] new commands for migration to base image #30520
Conversation
Current dependencies on/for this PR: This comment was auto-generated by Graphite. |
9029852
to
e6bcea4
Compare
9536d3f
to
7b65144
Compare
e6bcea4
to
fb1a22b
Compare
0ff5759
to
1d87e9a
Compare
fb1a22b
to
e3a6a77
Compare
a1b0d77
to
2d3ee2e
Compare
e3a6a77
to
fc5519a
Compare
2d3ee2e
to
f6e7ab5
Compare
fc5519a
to
981fc3e
Compare
f6e7ab5
to
4b23511
Compare
1de4960
to
1533e75
Compare
c69888e
to
0cf2c46
Compare
161d658
to
f5f8bd5
Compare
Tests are not running due to our current rate limiting issue. |
@@ -509,8 +516,10 @@ def list( | |||
|
|||
|
|||
@connectors.command(name="format", cls=DaggerPipelineCommand, help="Autoformat connector code.") | |||
@click.option("--commit-and-push", default=False) |
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.
❓I dont see this being used?
@click.pass_context | ||
def format_code(ctx: click.Context) -> bool: | ||
def format_code(ctx: click.Context, commit_and_push, export_to_host) -> bool: |
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 if we just remove this instead of update since were relying on gradlew format
atm
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.
Yes, these new options a remainder of an attempt to consolidate these command changing connector code and making them commit and push. I'll revert this. And will open a separate PR to remove format
"""Bump a connector version: update metadata.yaml, changelog and delete legacy files.""" | ||
|
||
connectors_contexts = [ | ||
ConnectorContext( |
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.
Do we need a whole connector context?
return context.report | ||
|
||
|
||
async def run_connector_migration_to_base_image_pipeline(context: ConnectorContext, semaphore, pull_request_number: str): |
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.
❓Why a pipeline and not a script?
Fine for it to stay as a pipeline but we likely want to add a note in a comment for when we can delete this code.
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.
I'm not sure this is a short lived command, as long as community connector use Dockerfile it has a reason to be considered a pipeline and not adhoc script.
@@ -38,3 +38,5 @@ | |||
DOCKER_HOST_NAME = "global-docker-host" | |||
DOCKER_HOST_PORT = 2375 | |||
DOCKER_TMP_VOLUME_NAME = "shared-tmp" | |||
REPO = git.Repo(search_parent_directories=True) |
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.
😍
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.
Few comments but nothing blocking
51a8fa1
to
16354fe
Compare
16354fe
to
e19a55b
Compare
Closes #30238
What
This PR adds useful
airbyte-ci
commands to make some connector changes:metadata.yaml
of the selected connectorairbyte-ci connectors --name=<my-connector> bump-version <{patch|minor|major> <PULL_REQUEST_NUMBER> <my changelog entry>
baseImage
to the latest version in themetadata.yaml
airbyte-ci connectors --name=<my-connector> upgrade-base-image
dockerfile / build.gradle
and update the connector documentation to share the new build instructions.airbyte-ci connectors --name=<my-connector> migrate-to-base-image <PULL_REQUEST_NUMBER>
Bonus
Delete the format command