Add --dry-run option to AssembleArgs to support non-destructive runs#5541
Add --dry-run option to AssembleArgs to support non-destructive runs#5541aravindkesavan wants to merge 1 commit intoopensearch-project:mainfrom
Conversation
Signed-off-by: Aravindan A <aravindkesavan07@gmail.com>
aravindkesavan
requested review from
Divyaasm,
gaiksaya,
peterzhuamazon,
prudhvigodithi,
rishabh6788,
tianleh and
zelinh
as code owners
|
@gaiksaya Could you please review this feature and provide your opinion on the next GO |
Will do in next few days. Just returned from my vacation today so catching up. |
peterzhuamazon
left a comment
peterzhuamazon
left a comment
There was a problem hiding this comment.
Thanks for contribution.
Tho I have a few doubt about this PR.
Thanks.
| version = args.version or "0.0.1" | ||
| dist = args.dist or "tar" | ||
|
|
||
| url = f"https://artifacts.opensearch.org/releases/bundle/{component}/{version}/{component}-{version}-{platform_name}-{arch}.{dist}" |
There was a problem hiding this comment.
We should not hard code this to prod url as if you see other similar features, we only use prod url substitutions in Jenkins lib. As opensearch-build was meant to run in any env and expected the files to be linked to local file paths.
| ```bash | ||
| python src/run_assemble.py <manifest-file> [options] | ||
| ``` |
There was a problem hiding this comment.
I do not think specifying this as we use the shell script as entrypoint with everything linked to pipenv.
This would add up confusions and remove the abstraction layer.
Users may not installed all the libs beforehand.
| #### CLI Flags | ||
|
|
||
| | Option | Description | | ||
| |--------------------------|-----------------------------------------------------------------------------| | ||
| | `--generate-manifest` | Generate a manifest YAML file dynamically (default filename: `manifest.yml`). | | ||
| | `--dry-run` | Simulate the assemble step without downloading or extracting artifacts. | | ||
| | `--version` | OpenSearch version (e.g., `2.14.0`). Used with `--generate-manifest`. | | ||
| | `--platform` | Target platform (`linux`, `windows`, `macos`). | | ||
| | `--arch` | Target architecture (`x64`, `arm64`). | | ||
| | `--dist` | Distribution type (`tar`, `zip`, `rpm`). | | ||
| | `--component` | Component name (defaults to `opensearch`). | | ||
| | `-b`, `--base-url` | Base URL to download artifacts from. | | ||
| | `--keep` | Do not delete the working temporary directory. | | ||
| | `-v`, `--verbose` | Enable verbose logging. | |
There was a problem hiding this comment.
These could be add in the similar chart above when discussing assemble.sh.
| #### Examples | ||
|
|
||
| **Assemble using an existing manifest:** | ||
|
|
||
| ```bash | ||
| python src/run_assemble.py builds/opensearch/manifest.yml --verbose | ||
| ``` | ||
|
|
||
| **Dry run (simulate without downloading or extracting):** | ||
|
|
||
| ```bash | ||
| python src/run_assemble.py builds/opensearch/manifest.yml --dry-run | ||
| ``` | ||
|
|
||
| **Generate a new manifest file for version 2.14.0:** | ||
|
|
||
| ```bash | ||
| python src/run_assemble.py --generate-manifest output-manifest.yml --version 2.14.0 --platform linux --arch x64 --dist tar --component opensearch | ||
| ``` |
There was a problem hiding this comment.
Same as above, can be added as examples for assemble.sh.
| console.configure(level=args.logging_level) | ||
|
|
||
| if args.dry_run: | ||
| print("Dry-run enabled. Skipping actual download or extraction.") |
There was a problem hiding this comment.
We should use logging instead of print.
There was a problem hiding this comment.
Similar for all the prints here.
| if args.manifest: | ||
| print(f"Manifest file: {args.manifest.name}") | ||
| else: | ||
| print("No manifest file provided.") | ||
| return 0 |
There was a problem hiding this comment.
Did not get, what is the usage of --dry-run here?
I can understand generate_manifest without actual assembling, but for dry_run itself it just print three times?
| manifest_content = { | ||
| "build": { | ||
| "name": component, | ||
| "version": version, | ||
| "platform": platform_name, | ||
| "architecture": arch, | ||
| "distribution": dist | ||
| }, | ||
| "components": [ | ||
| { | ||
| "name": component, | ||
| "version": version, | ||
| "url": url | ||
| } | ||
| ] | ||
| } |
There was a problem hiding this comment.
Are we not reusing the recorder logic for assemble_workflow?
github-project-automation
bot
moved this to 👀 In Review
in Engineering Effectiveness Board
3 participants
Description
This PR adds a
--dry-runoption to the assemble tool to support non-destructive runs, allowing users to simulate the assembly process without downloading or extracting artifacts. Additionally, it introduces the capability to generate a manifest file dynamically via the--generate-manifestoption.With these enhancements, users can:
Generate a Manifest File Dynamically
Run the following command to create a manifest file named
dry-run-manifest.ymlwith your desired parameters (all optional):Generate a new manifest file for version 2.14.0:
Using the below command to run the assemble tool in dry-run mode with the generated manifest:
Other enhancements include new command-line arguments to specify version, platform, architecture, distribution type, and component name for the manifest generation feature.
Issues Resolved
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.