-
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-21460 NPE at Show In Web Browser for OpenShift server if connection is not connected #912
Conversation
if(service == null) { | ||
throw new GetWelcomePageURLException("Service is missing."); | ||
} | ||
|
||
IProject project = service.getProject(); |
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.
Not specific to your pull, but there is no need to get a project to get the routes. You can accomplish this same call using the connection and retrieving the required project information by service#getNamespace(). This additionally saves you a REST call to load the project resource, and you could remove the throw from the null project all together.
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.
Thank you for the suggestion. Done.
f022ba7
to
531a422
Compare
} | ||
} catch (OpenShiftException e) { | ||
ExpressCoreActivator.pluginLog().logError(expectedConnectionProblem, e); | ||
throw new GetApplicationException(expectedConnectionProblem); |
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.
imho we should not swallow the root OpenShiftException but pass it to the new exception
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.
Ok, done. Now, this catch does not log the error, it is to be logged or shown in a message dialog or rethrown where this method is called.
8890f9d
to
bda0388
Compare
bda0388
to
ffe1ca2
Compare
ffe1ca2
to
e41aff1
Compare
…tion is not connected OpenShift core plugin has no ui to show dialog, so problem is reported with special exceptions to be displayed by client ui plugin.
ok, cool: no NPE, a dialog that tells me that the connection is not established yet. |
merging to jbosstools-4.3.x and master |
OpenShift core plugin has no ui to show dialog, so problem is reported with special exceptions
to be displayed by client ui plugin.