-
Notifications
You must be signed in to change notification settings - Fork 6k
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
[release-automation] Step to build update version binary into zip and store as artifact #45363
Conversation
Signed-off-by: khluu <[email protected]>
Signed-off-by: khluu <[email protected]>
Signed-off-by: khluu <[email protected]>
Signed-off-by: khluu <[email protected]>
Signed-off-by: khluu <[email protected]>
Signed-off-by: khluu <[email protected]>
Signed-off-by: khluu <[email protected]>
Signed-off-by: khluu <[email protected]>
Signed-off-by: khluu <[email protected]>
Tested locally with this artifact: https://buildkite.com/ray-project/release-automation/builds/486#018f7df0-673e-493d-a03d-b8534431c7b8 and running script worked (files were changed) |
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.
looking good, but a few questions for context
|
||
@click.command() | ||
@click.option("--root_dir", required=True, type=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.
what should be the root_dir
values provided by the users? is this script used somewhere and need to be updated?
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.
root_dir would be the path to Ray repository that user would want the changes to be on. For automation bot, it would clone Ray repo now and pass the path to the repo as root_dir here. If user ran the script locally, root_dir would be their Bazel workspace path.
job_env: forge | ||
commands: | ||
- bazel build --build_python_zip --incompatible_use_python_toolchains=false --python_path=python //ci/ray_ci/automation:update_version | ||
- cp bazel-bin/ci/ray_ci/automation/update_version.zip /artifact-mount/ |
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.
will you upload the binaries somewhere after this step? if so do you plan to automate that step too?
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 think the bot will just use buildkite API to download this from the pipeline.
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 binaries would not be uploaded anywhere else besides the build artifacts. And yes the automation bot would download this artifact using the Buildkite API (we already have a helper function just for that)
job_env: forge | ||
commands: | ||
- bazel build --build_python_zip --incompatible_use_python_toolchains=false --python_path=python //ci/ray_ci/automation:update_version | ||
- cp bazel-bin/ci/ray_ci/automation/update_version.zip /artifact-mount/ |
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 think the bot will just use buildkite API to download this from the pipeline.
bazel_workspace_dir = os.environ.get("BUILD_WORKSPACE_DIRECTORY", "") | ||
|
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 also make this still working? like can we use this as the default value, and errors when it is empty?
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 yes that should be the default behavior for running 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.
updated to use bazel workspace dir as default when root_dir
. If bazel workspace dir is empty, raise exception
Signed-off-by: khluu <[email protected]>
Signed-off-by: khluu <[email protected]>
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.
""" | ||
Update the version in the files to the specified version. | ||
""" | ||
main_version, java_version = get_current_version(bazel_workspace_dir) | ||
if not root_dir: | ||
root_dir = os.environ.get("BUILD_WORKSPACE_DIRECTORY", "") |
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.
nit: don't need the ""
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.
done
if not root_dir: | ||
root_dir = os.environ.get("BUILD_WORKSPACE_DIRECTORY", "") | ||
if not root_dir: | ||
raise Exception("Argument root_dir is not found.") |
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.
maybe say: "please specify --root_dir when not using bazelisk run
" ?
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.
done
Signed-off-by: khluu <[email protected]>
product
repoupdate_version
binary into a python zip file and upload it as an artifact inrelease-automation
pipelineroot_dir
as an arg forupdate_version
since automation is using this on a cloned Ray repo