-
Notifications
You must be signed in to change notification settings - Fork 55
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
JBIDE-21868 - Performance issue on Deploy Docker Image wizard #1030
Conversation
@fbricon @jcantrill could you please review this since you're far more intimate with the code and usecase? |
} | ||
final List<EnvironmentVariable> envVars = this.imageMeta.env().stream().map(env -> env.split("=")) |
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 possible that value of variable is empty: "name="? In this case "name=".split("=").length = 1 and next map() will throw array out of index.
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.
@scabanovich that's right ! Im adding .filter(env -> env.indexOf('=') != -1)
to filter out any env variable that would not match the expected name=value
pattern.
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.
@xcoulon , good that my comment helped you to find one more case to consider, but my idea was not that '=' is absent but that name=value has empty value so that literally string is 'name='. In this case split("=") returns array of length 1.
If this case can never happen, then there is nothing to do for it, but it is good that an incorrect question may help bring to light a real problem.
Any chance you could add more tests pretty please? |
Moving the code that pull image metadata (sometimes using the imagestream capabilities) when the user inputs the Image Name to the page validation phase, when the user clicks on the 'Next >' button. Also renamed some fields to reflect their type and UI name: - image -> imageName - name -> resourceName This also seems to fix JBIDE-21815 Added JUnit test to cover the Docker Image metadata initialization
@fbricon I just updated this pull request by taking all comments into account, including your kind request for JUnit tests ;-) |
Everything works fine for me. Applied in master / 4.3.x |
Great ! thanks, @fbricon ;-) |
Moving the code that pull image metadata (sometimes using the
imagestream capabilities) when the user inputs the Image Name
to the page validation phase, when the user clicks on the
'Next >' button.
Also renamed some fields to reflect their type and UI name:
This also seems to fix JBIDE-21815