You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2021/07/28 04:05:54 UTC
[GitHub] [nifi] exceptionfactory commented on a change in pull request #5250: NIFI-8785 - Confluent Schema Registry REST client - refactoring, debu…
exceptionfactory commented on a change in pull request #5250:
URL: https://github.com/apache/nifi/pull/5250#discussion_r677952002
##########
File path: nifi-nar-bundles/nifi-confluent-platform-bundle/nifi-confluent-schema-registry-service/src/main/java/org/apache/nifi/confluent/schemaregistry/client/RestSchemaRegistryClient.java
##########
@@ -196,6 +243,11 @@ private JsonNode postJsonResponse(final String pathSuffix, final JsonNode schema
final String path = getPath(pathSuffix);
final String trimmedBase = getTrimmedBase(baseUrl);
final String url = trimmedBase + path;
+
+ if(logger.isDebugEnabled()) {
+ logger.debug("POST JSON response from " + url);
+ }
Review comment:
This logging statement could be adjusted to use placeholders, which would also remove the need to wrap it in the conditional.
```suggestion
logger.debug("POST JSON response URL {}", url);
```
##########
File path: nifi-nar-bundles/nifi-confluent-platform-bundle/nifi-confluent-schema-registry-service/src/main/java/org/apache/nifi/confluent/schemaregistry/client/RestSchemaRegistryClient.java
##########
@@ -232,20 +301,29 @@ private JsonNode fetchJsonResponse(final String pathSuffix, final String schemaD
final Response response = builder.get();
final int responseCode = response.getStatus();
- if (responseCode == Response.Status.OK.getStatusCode()) {
- return response.readEntity(JsonNode.class);
- }
+ switch (Response.Status.fromStatusCode(responseCode)) {
+ case OK:
+ JsonNode jsonResponse = response.readEntity(JsonNode.class);
- if (responseCode == Response.Status.NOT_FOUND.getStatusCode()) {
- throw new SchemaNotFoundException("Could not find Schema with " + schemaDescription + " from the Confluent Schema Registry located at " + baseUrl);
- }
+ if(logger.isDebugEnabled()) {
+ logger.debug("Response: " + jsonResponse.toString());
+ }
+
+ return jsonResponse;
- if (errorMessage == null) {
- errorMessage = response.readEntity(String.class);
+ case NOT_FOUND:
+ logger.debug("Could not find Schema with " + schemaDescription
+ + " from the Confluent Schema Registry located at " + baseUrl);
Review comment:
```suggestion
logger.debug("Could not find Schema {} from Registry {}", schemaDescription, baseUrl);
```
##########
File path: nifi-nar-bundles/nifi-confluent-platform-bundle/nifi-confluent-schema-registry-service/src/main/java/org/apache/nifi/confluent/schemaregistry/client/RestSchemaRegistryClient.java
##########
@@ -232,20 +301,29 @@ private JsonNode fetchJsonResponse(final String pathSuffix, final String schemaD
final Response response = builder.get();
final int responseCode = response.getStatus();
- if (responseCode == Response.Status.OK.getStatusCode()) {
- return response.readEntity(JsonNode.class);
- }
+ switch (Response.Status.fromStatusCode(responseCode)) {
+ case OK:
+ JsonNode jsonResponse = response.readEntity(JsonNode.class);
- if (responseCode == Response.Status.NOT_FOUND.getStatusCode()) {
- throw new SchemaNotFoundException("Could not find Schema with " + schemaDescription + " from the Confluent Schema Registry located at " + baseUrl);
- }
+ if(logger.isDebugEnabled()) {
+ logger.debug("Response: " + jsonResponse.toString());
+ }
Review comment:
```suggestion
if (logger.isDebugEnabled()) {
logger.debug("JSON Response {}", jsonResponse);
}
```
##########
File path: nifi-nar-bundles/nifi-confluent-platform-bundle/nifi-confluent-schema-registry-service/src/main/java/org/apache/nifi/confluent/schemaregistry/client/RestSchemaRegistryClient.java
##########
@@ -204,26 +256,43 @@ private JsonNode postJsonResponse(final String pathSuffix, final JsonNode schema
final Response response = builder.post(Entity.json(schema.toString()));
final int responseCode = response.getStatus();
- if (responseCode == Response.Status.NOT_FOUND.getStatusCode()) {
- continue;
- }
+ switch (Response.Status.fromStatusCode(responseCode)) {
+ case OK:
+ JsonNode jsonResponse = response.readEntity(JsonNode.class);
+
+ if(logger.isDebugEnabled()) {
+ logger.debug("Response: " + jsonResponse.toString());
+ }
- if(responseCode == Response.Status.OK.getStatusCode()) {
- return response.readEntity(JsonNode.class);
+ return jsonResponse;
+
+ case NOT_FOUND:
+ logger.debug("Could not find Schema with " + schemaDescription
+ + " from the Confluent Schema Registry located at " + baseUrl);
Review comment:
```suggestion
logger.debug("Could not find Schema {} from Registry {}", schemaDescription, baseUrl);
```
##########
File path: nifi-nar-bundles/nifi-confluent-platform-bundle/nifi-confluent-schema-registry-service/src/main/java/org/apache/nifi/confluent/schemaregistry/client/RestSchemaRegistryClient.java
##########
@@ -204,26 +256,43 @@ private JsonNode postJsonResponse(final String pathSuffix, final JsonNode schema
final Response response = builder.post(Entity.json(schema.toString()));
final int responseCode = response.getStatus();
- if (responseCode == Response.Status.NOT_FOUND.getStatusCode()) {
- continue;
- }
+ switch (Response.Status.fromStatusCode(responseCode)) {
+ case OK:
+ JsonNode jsonResponse = response.readEntity(JsonNode.class);
+
+ if(logger.isDebugEnabled()) {
+ logger.debug("Response: " + jsonResponse.toString());
+ }
- if(responseCode == Response.Status.OK.getStatusCode()) {
- return response.readEntity(JsonNode.class);
+ return jsonResponse;
+
+ case NOT_FOUND:
+ logger.debug("Could not find Schema with " + schemaDescription
+ + " from the Confluent Schema Registry located at " + baseUrl);
+ continue;
+
+ default:
+ errorMessage = response.readEntity(String.class);
+ continue;
}
}
- throw new SchemaNotFoundException("Failed to retrieve Schema with " + schemaDescription + " from any of the Confluent Schema Registry URL's provided; failure response message: "
+ throw new SchemaNotFoundException("Failed to retrieve Schema with " + schemaDescription
+ + " from any of the Confluent Schema Registry URL's provided; failure response message: "
+ errorMessage);
}
- private JsonNode fetchJsonResponse(final String pathSuffix, final String schemaDescription) throws SchemaNotFoundException, IOException {
+ private JsonNode fetchJsonResponse(final String pathSuffix, final String schemaDescription) throws SchemaNotFoundException {
String errorMessage = null;
for (final String baseUrl : baseUrls) {
final String path = getPath(pathSuffix);
final String trimmedBase = getTrimmedBase(baseUrl);
final String url = trimmedBase + path;
+ if(logger.isDebugEnabled()) {
+ logger.debug("GET JSON response from " + url);
+ }
Review comment:
```suggestion
logger.debug("GET JSON response URL {}", url);
```
##########
File path: nifi-nar-bundles/nifi-confluent-platform-bundle/nifi-confluent-schema-registry-service/src/main/java/org/apache/nifi/confluent/schemaregistry/client/RestSchemaRegistryClient.java
##########
@@ -204,26 +256,43 @@ private JsonNode postJsonResponse(final String pathSuffix, final JsonNode schema
final Response response = builder.post(Entity.json(schema.toString()));
final int responseCode = response.getStatus();
- if (responseCode == Response.Status.NOT_FOUND.getStatusCode()) {
- continue;
- }
+ switch (Response.Status.fromStatusCode(responseCode)) {
+ case OK:
+ JsonNode jsonResponse = response.readEntity(JsonNode.class);
+
+ if(logger.isDebugEnabled()) {
+ logger.debug("Response: " + jsonResponse.toString());
+ }
Review comment:
Conditional logging seems reasonable here given the serialization of the JsonNode, but using placeholders would be helpful.
```suggestion
if (logger.isDebugEnabled()) {
logger.debug("JSON Response: {}", jsonResponse);
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org