-
Notifications
You must be signed in to change notification settings - Fork 312
Add --dry-run option to AssembleArgs to support non-destructive runs #5541
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
base: main
Are you sure you want to change the base?
Add --dry-run option to AssembleArgs to support non-destructive runs #5541
Conversation
Signed-off-by: Aravindan A <[email protected]>
@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. |
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 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use logging instead of print.
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.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we not reusing the recorder logic for assemble_workflow?
Description
This PR adds a
--dry-run
option 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-manifest
option.With these enhancements, users can:
Generate a Manifest File Dynamically
Run the following command to create a manifest file named
dry-run-manifest.yml
with 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.