-
Notifications
You must be signed in to change notification settings - Fork 80
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
Rename provision config instance #400
Rename provision config instance #400
Conversation
osbenchmark/benchmark.py
Outdated
default="default") | ||
p.add_argument( | ||
"--provision-config-revision", | ||
help="Define a specific revision in the provision_config repository that Benchmark should use.", | ||
help="Define a specific revision in the cluster_configs repository that Benchmark should use.", |
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: cluster_configs
--> cluster-configs
to keep consistency in the help definitions
osbenchmark/benchmark.py
Outdated
f"provision_config_instances with `{PROGRAM_NAME} list " | ||
f"provision_config_instances` (default: defaults).", | ||
"--cluster-config", | ||
help=f"Define the cluster_config to use. List possible " |
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: Update cluster_config
--> cluster-config
to keep consistency in help texts
osbenchmark/benchmark.py
Outdated
"--provision-config-instance-params", | ||
help="Define a comma-separated list of key:value pairs that are injected verbatim as variables for the provision_config_instance.", | ||
"--cluster-config-params", | ||
help="Define a comma-separated list of key:value pairs that are injected verbatim as variables for the cluster_config.", |
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: cluster_config
--> cluster-config
osbenchmark/benchmark.py
Outdated
"--cluster-config", | ||
help=f"Define the cluster_config to use. List possible " | ||
f"cluster_configs with `{PROGRAM_NAME} list " | ||
f"cluster_configs` (default: defaults).", |
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: cluster_configs
--> cluster-configs
osbenchmark/benchmark.py
Outdated
f"provision_config_instances with `{PROGRAM_NAME} list " | ||
f"provision_config_instances` (default: defaults).", | ||
"--cluster-config", | ||
help=f"Define the cluster_config to use. List possible " |
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: cluster_config
and cluster_configs
--> cluster-config
and cluster-configs
osbenchmark/benchmark.py
Outdated
default="default") | ||
install_parser.add_argument( | ||
"--provision-config-revision", | ||
help="Define a specific revision in the provision_config repository that Benchmark should use.", | ||
default=None) | ||
install_parser.add_argument( | ||
"--provision-config-path", | ||
help="Define the path to the provision_config_instance and plugin configurations to use.") | ||
help="Define the path to the cluster_config and plugin configurations to use.") |
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: cluster_config
--> cluster-config
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.
Left some minor suggestions but overall looks good to me! Were you able to test this? Could you provide a sample output of the help text and a brief run with a cluster-config to ensure that it works?
When we merge this in, we'll merge this into a separate branch first since these changes are considered breaking-changes. Thus, we don't want to merge this into mainline yet until we're ready to release OSB 2.0.0
Yes, I will make the suggested changes and share output
…On Tue, Oct 31, 2023, 9:18 AM Ian Hoang ***@***.***> wrote:
***@***.**** requested changes on this pull request.
Left some minor suggestions but overall looks good to me! Were you able to
test this? Could you provide a sample output of the help text and a brief
run with a cluster-config to ensure that it works?
When we merge this in, we'll merge this into a separate branch first since
these changes are considered breaking-changes. Thus, we don't want to merge
this into mainline yet until we're ready to release OSB 2.0.0
—
Reply to this email directly, view it on GitHub
<#400 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABUCKL7KFHNA6SWEC7NRW7TYCEQE3AVCNFSM6AAAAAA6T2DUD2VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMBWG4ZTONRSG4>
.
You are receiving this because you authored the thread.Message ID:
***@***.***
.com>
|
Signed-off-by: Chinmay Gadgil <[email protected]> renamed provision_config_instances directory Signed-off-by: Chinmay Gadgil <[email protected]> renamed pending provision_config_instance to cluster_config Signed-off-by: Chinmay Gadgil <[email protected]> renamed provision-config-path,repostiry and revision to cluster-config-path,repository and revision respectively renamed provision_config_instance_types to cluster_config_types Signed-off-by: Chinmay Gadgil <[email protected]> renamed provision_config_instance_descriptor to cluster_config_descriptor Signed-off-by: Chinmay Gadgil <[email protected]>
49c219b
to
385ff96
Compare
a23c0a5
into
opensearch-project:renaming-components
Signed-off-by: Chinmay Gadgil <[email protected]>
Description
[Describe what this change achieves]
renames provision_config_instance to cluster-config
Issues Resolved
[List any issues this PR will resolve]
#310
Testing
[Describe how this change was tested]
validated with
make test
andmake it
Running help with list command shows the renamed options
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.