-
Notifications
You must be signed in to change notification settings - Fork 24.9k
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
Add EC2 credential test for repository-s3 #31918
Changes from 14 commits
0278c88
a56d1cd
5fe4227
30e86c8
46dded9
ccfa0c5
c7af0b6
0d31887
fd28579
4006d19
a4b3613
9c4c3e7
5430473
0ffae39
161deee
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 |
---|---|---|
|
@@ -180,15 +180,7 @@ class NodeInfo { | |
} | ||
|
||
args.addAll("-E", "node.portsfile=true") | ||
String collectedSystemProperties = config.systemProperties.collect { key, value -> "-D${key}=${value}" }.join(" ") | ||
String esJavaOpts = config.jvmArgs.isEmpty() ? collectedSystemProperties : collectedSystemProperties + " " + config.jvmArgs | ||
if (Boolean.parseBoolean(System.getProperty('tests.asserts', 'true'))) { | ||
// put the enable assertions options before other options to allow | ||
// flexibility to disable assertions for specific packages or classes | ||
// in the cluster-specific options | ||
esJavaOpts = String.join(" ", "-ea", "-esa", esJavaOpts) | ||
} | ||
env = ['ES_JAVA_OPTS': esJavaOpts] | ||
env = [:] | ||
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. this can be removed, the map is already initialized. 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. nope, map is
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. My bad, this class initializes in the constructor, this is ok as it is. |
||
for (Map.Entry<String, String> property : System.properties.entrySet()) { | ||
if (property.key.startsWith('tests.es.')) { | ||
args.add("-E") | ||
|
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 difference is easy to overlook , maybe it would be good to have a comment explaining why
Object
is needed.We might be able to have
Map<String, GString>
to make the intention cleaner.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.
💯