-
Notifications
You must be signed in to change notification settings - Fork 690
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
[JENKINS-61161] Avoid infamous UI connection strategy warning #538
Conversation
@@ -2040,7 +2040,7 @@ public ListBoxModel doFillConnectionStrategyItems(@QueryParameter String connect | |||
|
|||
public FormValidation doCheckConnectionStrategy(@QueryParameter String connectionStrategy) { | |||
return Stream.of(ConnectionStrategy.values()) | |||
.filter(v -> v.equalsName(connectionStrategy)) | |||
.filter(v -> v.name().equals(connectionStrategy)) |
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.
- Previously:
Public_IP == PUBLIC_IP
?? - Now:
PUBLIC_IP == PUBLIC_IP
??
See:
PUBLIC_IP("Public IP"), |
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 it not the same? why is it fixing the error?
ec2-plugin/src/main/java/hudson/plugins/ec2/ConnectionStrategy.java
Lines 15 to 17 in 2f348aa
public boolean equalsName(String other) { | |
return this.name.equals(other); | |
} |
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 too am interested in Francisco's point
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.
equalsName
is comparing the text added in the constructor (Public IP
lowercase) as you can see above, but the connectionStrategy
variable contains the declared name of the enum (PUBLIC IP
uppercase), so they were never equals. With name()
we use the declared name.
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.
Oh! I see now! Really good point Ramon
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.
Interesting, then shouldn't equals()
also produce the desired behaviour?
Edit: Forgot the the other object is simply a string, LGTM. thanks for the explaination
@@ -2040,7 +2040,7 @@ public ListBoxModel doFillConnectionStrategyItems(@QueryParameter String connect | |||
|
|||
public FormValidation doCheckConnectionStrategy(@QueryParameter String connectionStrategy) { | |||
return Stream.of(ConnectionStrategy.values()) | |||
.filter(v -> v.equalsName(connectionStrategy)) | |||
.filter(v -> v.name().equals(connectionStrategy)) |
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.
Oh! I see now! Really good point Ramon
Regression? I have this error again in EC2 plugin version 1.56 |
I can corroborate - this is still an issue. We use CasC and it will not work under any condition. The knock on effect is that after a configuration update and restart of Jenkins, no EC2 instance can be created. Perhaps the logic of this validation function is flawed? Frankly I do not mind seeing the actual enum value show up in as the form value |
This is still an issue in 1.59 🤕 @amedee, @jensenbox Have you guys found any intermediate solution to this? Doing "Apply" on the Clouds configuration from the UI "fixes" the Connection Strategy problem. Having to |
I can recreate the issue when adding a new template which is fixed in #626 |
@jensenbox See this comment in case you're still facing the JCasC issue. 👍 |
As of 8 hours and 39 minutes ago, I no longer work at the company where
we had this issue.
I no longer have access to the Jenkins there so I can't check the
configuration.
I am unable to help you, sorry.
…On 2/06/2021 11:30, radnov wrote:
This is still an issue in 1.59 🤕
@amedee <https://github.com/amedee>, @jensenbox
<https://github.com/jensenbox> Have you guys found any intermediate
solution to this?
Doing "Apply" on the Clouds configuration from the UI "fixes" the
Connection Strategy problem. Having to |POST
$JENKINS_URL/configureClouds/configure| on each JCasC reload is just
nasty.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#538 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AADRIWPXNBK7HFDAGMIVH5DTQX23PANCNFSM4UA4KZEA>.
|
This is more broken now with 1.60 than before. I literally tried every possible combination of fields but the connection strategy always falls back to "Public DNS". |
Fixed in #627 has to do with a weird issue where the name of the variable was causing issues. I tested it locally and it seems to be fine now even when saving and loading. Will be making a new release 1.60.1 for this |
See https://issues.jenkins.io/browse/JENKINS-61161
Avoid the ugly and erroneous Could not find selected connection strategy every time one changes the connection strategy.
See:
![Screenshot from 2020-11-24 14-16-51](https://user-images.githubusercontent.com/22069922/100100350-f8bff580-2e60-11eb-95f0-35eda27bfc73.png)