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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
