Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
workflow for publishing artifacts for cloud #14199
workflow for publishing artifacts for cloud #14199
Changes from all commits
834ae88
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
might be missing something, why move the jar publishing to be before the Docker image publishing?
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 doesn't actually matter, but I just wanted to try splitting up the steps into different jobs. To do this, the publish step depends on some java setup steps in the Build Branch step, so I had to keep these within the same job, whereas the docker stuff I could split out into a separate job.
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.
understand, thanks.
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.
remind me again why we are publishing master jars with this if condition?
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 in the tech spec https://docs.google.com/document/d/1Os-GMcDwJuqVNKI8yUmSbOevYt92N1W3BWY-Dj8DqdY/edit#heading=h.pyzrac8d5gsg where commits from the master branch would create different tags/jar versions from non master commits. I think we can actually drop this feature though because I don't actually see how we'd use these tags. I think it'd be easier to just assume the tag will always be
dev-<short_sha>
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.
Thanks Peter. I do think it's useful though agree this is an incremental. I think it's worth adding a comment here explaining why. also think it's fine to remove if it seems confusing.
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.
yeah, I think we can just leave it as is for now, but if publishing master jars seems to noticeably slow down this workflow then we can probably remove this step and comment on why
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.
ideally we could just modify the existing
docker-compose-cloud.build.yaml
instead of creating a new (mostly duplicate) file here. However, the workflow runner amis contain an outdated version of docker-compose that errors out when encounteringx-
fields.Based on the latest spec, docker-compose is supposed to ignore all unknown extensions without failing (and this is the case when running docker-compose locally).
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.
can we remove the old docker-compose build yaml file?
from my quick search, I think we should be able to.
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.
The old yaml is used by the build-and-push-branch action which is called from deploy-command.yml in cloud
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.
right. I should be clearer. I mean if the docker compose ignores unknown extensions, can we replace all usages of the old docker compose file with the new buildx one?
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.
oh right yes, if we can update docker compose to ignore the unknown extensions then we can replace all usages of the old compose file with this new one
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.
yeap. can we either create an issue or note this down somewhere so we don't forget?
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.
created https://github.com/airbytehq/airbyte-cloud/issues/1949