-
Notifications
You must be signed in to change notification settings - Fork 4.1k
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: productionize up-to-date 🧹 + 🚀 #39600
airbyte-ci: productionize up-to-date 🧹 + 🚀 #39600
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @alafanechere and the rest of your teammates on Graphite |
c7631da
to
b8aba10
Compare
9c874f5
to
eb5b545
Compare
@@ -58,11 +58,11 @@ def download_catalog(catalog_url): | |||
|
|||
|
|||
OSS_CATALOG = download_catalog(OSS_CATALOG_URL) | |||
METADATA_FILE_NAME = "metadata.yaml" |
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.
It was declared twice
@@ -58,7 +56,6 @@ async def bump_version( | |||
ctx.obj["execute_timeout"], | |||
bump_type, | |||
changelog_entry, | |||
pull_request_number, |
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.
Going "unix style" here: the bump command just bumps version / changelog, it does not open PRs; the pull requests command can be used sequentially following a version bump.
from pipelines.models.steps import Step, StepResult, StepStatus | ||
|
||
if TYPE_CHECKING: | ||
from anyio import Semaphore | ||
|
||
|
||
def get_bumped_version(version: str | None, bump_type: str) -> 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.
Moved this to the BumpConnectorVersion
step
@@ -79,202 +60,11 @@ async def _cleanup(self) -> StepResult: | |||
return StepResult(step=self, status=StepStatus.SUCCESS) | |||
|
|||
|
|||
class AddChangelogEntry(Step): |
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.
Moved this to pipelines.airbyte_ci.steps.changelog
as it's a step reused between multiple commands.
) | ||
|
||
|
||
class SetConnectorVersion(Step): |
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.
Moved to pipelines.airbyte_ci.steps.bump_version
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.
Adopting consistent kebab casing for connectors subcommands
pull_request_number, | ||
changelog, | ||
bump, |
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.
Going unix style: the bump and pull request pr can be used after this command if needed.
|
||
from anyio import Semaphore | ||
|
||
|
||
class UpgradeBaseImageMetadata(Step): |
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.
Moved to pipelines.airbyte_ci.steps.base_image
6af37db
to
f648f51
Compare
51a7203
to
e431f7b
Compare
changelog.add_entry(semver.VersionInfo.parse("3.4.0"), datetime.date.fromisoformat("2024-03-01"), 123456, "test1") | ||
changelog.add_entry(semver.VersionInfo.parse("3.4.0"), datetime.date.fromisoformat("2024-03-01"), 123457, "test2") | ||
check_result(changelog, "dupicate_version_date_" + filename) | ||
# This test is disabled because the current implementation allows inserting non number PR numbers. |
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.
I made the command name casing consistent: use kebab-case everywhere instead of a mix of kebab and snake.
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.
Import cleaning following poe lint --fix
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.
Simplified this command: it does not bump the connector version. Use bump-version
command for that
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.
This was a set of checks performed on the connector following updates. Now that updates are automated I believe it's CI role to validate the updates are valid, it should not be done by the up-to-date pipeline itself
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.
Deleted as upgrade_base_image
is now part of the up-to-date
command
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.
Deleted as upgrade_base_image
is now part of the up-to-date
command
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.
Deleted as upgrade_base_image
is now part of the up-to-date
command
e431f7b
to
edc9da8
Compare
a0f72b1
to
e781c80
Compare
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.
exciting. I'm a bit curious to see it to:
- make a PR
- master changes in lots of ways
- update the PR
I expected to see something like a force push or using the master branch as the base_ref
in the PR code. I had some crazy-long comment about that before that I can't find.
Anyway, be on the lookout for that.
@bleonard I removed it because I "resolved" it:
|
e781c80
to
dc398e9
Compare
What
Closes https://github.com/airbytehq/airbyte-internal-issues/issues/8319
Closes https://github.com/airbytehq/airbyte-internal-issues/issues/7418
👀 : A loom is worth a thousand words.
We originally had multiple command we used manually and locally to ship mass connector updates.
These commands had optional bumping/PR creation logic with a lot of shared but duplicate logic.
This PR cleans things up and productionizes the
up-to-date
command so that it can run automatically in a cronjob.How
migrate-to-poetry
) and then executebump-version
andpull-request
commands to update the connector versions and open a PR. This allows keeping "simple" and isolated steps with less potential side effects.up-to-date
command is meant to be automated in GHA this command is not "unix style". It:poetry update
to update Python dependenciesup-to-date/<connector-name>
branch and force pushes the modified files to it. (CI disabled)auto-merge
label if the--auto-merge
option is passed).Review guide
* The
up-to-date
pipeline logic is the most important part of this PR. Everything else is basically code cleaning and slight re-architecture.auto-merge
daily run.Demo
I manually ran the workflow from this branch. (It's not done yet and I'll monitor if it can correctly open PRs for our 300 connectors in a reasonable amount of time).
It created these PRs.
I disabled the
auto-merge
label for testing.User Impact
Not much. Some command names (using kebab case) and options changed but the documentation has been updated.
Can this PR be safely reverted and rolled back?