-
Notifications
You must be signed in to change notification settings - Fork 314
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
Allow to use the bundled JDK in Elasticsearch #853
Changes from all commits
ab9391c
ba03833
9a07f66
6662420
449dbf4
70b218d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,9 +35,18 @@ def create_arg_parser(): | |
def positive_number(v): | ||
value = int(v) | ||
if value <= 0: | ||
raise argparse.ArgumentTypeError("must be positive but was %s" % value) | ||
raise argparse.ArgumentTypeError("must be positive but was {}".format(value)) | ||
return value | ||
|
||
def runtime_jdk(v): | ||
if v == "bundled": | ||
return v | ||
else: | ||
try: | ||
return positive_number(v) | ||
except argparse.ArgumentTypeError: | ||
raise argparse.ArgumentTypeError("must be a positive number or 'bundled' but was {}".format(v)) | ||
|
||
# try to preload configurable defaults, but this does not work together with `--configuration-name` (which is undocumented anyway) | ||
cfg = config.Config() | ||
if cfg.config_present(): | ||
|
@@ -187,6 +196,9 @@ def positive_number(v): | |
"--team-repository", | ||
help="Define the repository from where Rally will load teams and cars (default: default).", | ||
default="default") | ||
download_parser.add_argument( | ||
"--team-path", | ||
help="Define the path to the car and plugin configurations to use.") | ||
download_parser.add_argument( | ||
"--distribution-version", | ||
help="Define the version of the Elasticsearch distribution to download. " | ||
|
@@ -205,6 +217,14 @@ def positive_number(v): | |
help="Define a comma-separated list of key:value pairs that are injected verbatim as variables for the car.", | ||
default="" | ||
) | ||
download_parser.add_argument( | ||
"--target-os", | ||
help="The name of the target operating system for which an artifact should be downloaded (default: current OS)", | ||
) | ||
download_parser.add_argument( | ||
"--target-arch", | ||
help="The name of the CPU architecture for which an artifact should be downloaded (default: current architecture)", | ||
) | ||
|
||
install_parser = subparsers.add_parser("install", help="Installs an Elasticsearch node locally") | ||
install_parser.add_argument( | ||
|
@@ -303,7 +323,7 @@ def positive_number(v): | |
default="") | ||
start_parser.add_argument( | ||
"--runtime-jdk", | ||
type=positive_number, | ||
type=runtime_jdk, | ||
help="The major version of the runtime JDK to use.", | ||
default=None) | ||
start_parser.add_argument( | ||
|
@@ -338,7 +358,7 @@ def positive_number(v): | |
default="") | ||
p.add_argument( | ||
"--runtime-jdk", | ||
type=positive_number, | ||
type=runtime_jdk, | ||
help="The major version of the runtime JDK to use.", | ||
default=None) | ||
|
||
|
@@ -850,6 +870,9 @@ def main(): | |
console.println("--target-hosts and --client-options must define the same keys for multi cluster setups.") | ||
exit(1) | ||
# split by component? | ||
if sub_command == "download": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don't we need to support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would like to restrict this to the |
||
cfg.add(config.Scope.applicationOverride, "mechanic", "target.os", args.target_os) | ||
cfg.add(config.Scope.applicationOverride, "mechanic", "target.arch", args.target_arch) | ||
if sub_command == "list": | ||
cfg.add(config.Scope.applicationOverride, "system", "list.config.option", args.configuration) | ||
cfg.add(config.Scope.applicationOverride, "system", "list.races.max_results", args.limit) | ||
|
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.
Is there a reason -- apart from simplicity -- we can't use j2 template rendering to substitute those (and at the same time be more lenient with spaces like
{{ VERSION }}
and allow filters)? Many other files in rally-teams are treated as j2 hence the question.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.
For other templates (like
elasticsearch.yml
orjvm.options
) I think Jinja makes sense because their purpose is to serve as templates.config.ini
is a high-level file meant to control how Rally behaves and I would not expect much templating happening here. As discussed elsewhere, I'll raise a separate enhancement issue.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've raised #856.