-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Testclusters: convert left-overs from checkPart1 #43370
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
Conversation
|
Pinging @elastic/es-core-infra |
| autoSetInitialMasterNodes = false | ||
| autoSetHostsProvider = false | ||
| testClusters.integTest { | ||
| nameCustomization = { null } |
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 way to do this in a more first-class way instead of using a null node name? The node name actually isn't relevant here. We just want to disable discovery. Perhaps we should make that a formal option on TestClusterConfiguration? We can then remove all the handling for null node names which is confusing IMO.
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 do also care about not configuring a node name for the test. We are also testing that we can start up without it. I taught about having a more formal way to do this, but this is the only test that would use it and it's a bit of an edge case.
Would definitely consider extending this if we were to have other users.
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.
Ah ok. I assumed the empty node name was just an implementation detail.
|
@mark-vieira are you ok with this PR now ? |
|
Yup 👍 |
* Testclusters: convert left-overs from checkPart1
A few projects of checkPart1 were not converted. This PR closes that gap.