-
Notifications
You must be signed in to change notification settings - Fork 36
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
Upload dependencies to S3 bucket on new tag release #689
Upload dependencies to S3 bucket on new tag release #689
Conversation
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 changes in this PR look good - nice work.
#685 makes me a little nervous - IIUC we could recreate the tag whenever a change is made to version.py, which seems a little too implicit for my liking.
I think we should make sure some tests are run before releasing any new artifacts. That should probably be done before tagging.
- name: Get version number | ||
shell: bash | ||
run: | | ||
VERSION=$(grep -oE '[0-9]+\.[0-9]+\.[0-9]+(\-[a-zA-Z]+[0-9]+)?' share/version.py) |
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.
Perhaps get the tag from git, to avoid duplicating this logic across multiple workflows? e.g. we could use git describe
to get the tag for the current branch.
Indeed, I noticed it too, but I figured there is really no reason to change that file unless to update the version.
What kind of tests? There is the test workflow ( I don't mind recreating the PR to make a few changes @axw |
I actually made a mistake on the workflow to release a new tag, so I have to create a new PR on it anyway @axw. The PR is this one #690. Can you post your comments there on:
|
Ah I see. Is that guaranteed to have run before the release workflow? I guess that's OK to start with, but ideally we would run some smoke tests too, preferably before even tagging. But anyway that can come in a followup. For reference, apm-aws-lambda runs smoke tests on every push to main: https://github.com/elastic/apm-aws-lambda/blob/main/.github/workflows/smoke-tests.yml. Maybe a little excessive, but there might be some workflows and code that could be reused. |
Yes, it runs on every PR. The release only works on a push, so only after the tests. |
What does this PR do?
Uploads dependencies to S3 bucket on new tag release.
This workflow is dependent on workflow
release
- that is, it waits for the new release to zip all the dependencies and send it to S3 bucket.You can see more details discussed in issue #683.
Checklist
CHANGELOG.md
Related issues
Closes #683.