HWKMETRICS-728: fetch the namespace id directly if a miss in the namespace listener #875
Conversation
|
@jsanda @stefannegrea can one of you please take a look? |
| @@ -70,7 +71,61 @@ public void stop() { | |||
| * @return The id of the namespace or null if the namespace does not exist | |||
| */ | |||
| public String getNamespaceId(String namespaceName) { | |||
| return namespaces.get(namespaceName); | |||
| String namespaceId = namespaces.get(namespaceName); | |||
There was a problem hiding this comment.
computeIfAbsent would reduce possible multiple queries to the Kubernetes master,
| connection.setRequestProperty("Accept", "application/json"); | ||
| connection.setRequestProperty("Authorization", "Bearer " + token); | ||
|
|
||
| connection.connect(); |
There was a problem hiding this comment.
No connection.disconnect() ? Also, inputStream.close() at line 117 is not necessarily called, leaving handler open.
| inputStream.close(); | ||
|
|
||
| } else { | ||
| log.infof("Error getting project metadata. Response code %s", responseCode); |
There was a problem hiding this comment.
We're not interested in the error itself with .getErrorStream() ? Or is there any possible payload?
There was a problem hiding this comment.
Normally we are not all that interested with the payload. The messages we are more going to be getting here are going to be related to permissions. For now I think just the error code/message is what we need.
| } else { | ||
| log.info("Could not determine a namespace id for namespace " + namespaceID + ". Cannot process request. Returning an INTERNAL_SERVER_ERROR."); | ||
| log.info("Could not determine a namespace id for namespace. Cannot process request. Returning an INTERNAL_SERVER_ERROR."); |
There was a problem hiding this comment.
log.info but returning code 500?
There was a problem hiding this comment.
In the original PR I believe I was told to only use log.info here.
The response to the user has the real error message for this.
Would you prefer this to be another error type? or to have the log be at the warn/error level?
|
PR updated with requested changes. Will squash when finally ready. |
| jsonParser.close(); | ||
| } | ||
| if (inputStream != null) { | ||
| inputStream.close(); |
There was a problem hiding this comment.
This could still be left open, if jsonParser throws an Exception
mwringe
force-pushed
the
0.27-HWKMETRICS-728
branch
from
48b960e
to
b82466d
Compare
|
Updated and squashed |
|
Thank you @mwringe! |
See https://issues.jboss.org/browse/HWKMETRICS-728