tolbertam commented on code in PR #1949:
URL:
https://github.com/apache/cassandra-java-driver/pull/1949#discussion_r1740188182
##########
core/revapi.json:
##########
@@ -6956,6 +6956,16 @@
"old": "method java.lang.Throwable
java.lang.Throwable::fillInStackTrace() @
com.fasterxml.jackson.databind.deser.UnresolvedForwardReference",
"new": "method
com.fasterxml.jackson.databind.deser.UnresolvedForwardReference
com.fasterxml.jackson.databind.deser.UnresolvedForwardReference::fillInStackTrace()",
"justification": "Upgrade jackson-databind to 2.13.4.1 to address
CVEs, API change cause:
https://github.com/FasterXML/jackson-databind/issues/3419";
+ },
+ {
+ "code": "java.method.addedToInterface",
Review Comment:
I suppose this would not be a backwards compatible change that we are making
in a minor release, which [would break the documented following of
semver](https://github.com/apache/cassandra-java-driver/blob/a17f7be614a09ab81bc2982b7f7ab3a123b4ab28/manual/api_conventions/README.md?plain=1#L29-L31).
I think it would probably be ok to just make default implementations that
return `null`, add a `@Nullable` annotation, and document that callers should
consider use of a custom `ExecutionInfo`. We could mark removing the default
implementation for whenever we next do a major release.
I think this would be pragmatic as it would avoid possibly breaking any
third party libraries and requiring another release of the library that the
user may not have control of. I'll admit that this seems rather unlikely but I
think we should do everything we can to keep semver even if it is somewhat
non-optimal.
##########
core/src/main/java/com/datastax/oss/driver/internal/core/cql/CqlRequestHandler.java:
##########
@@ -368,7 +371,8 @@ private void setFinalResult(
logServerWarnings(callback.statement, executionProfile,
executionInfo.getWarnings());
}
} catch (Throwable error) {
- setFinalError(callback.statement, error, callback.node, -1);
+ ExecutionInfo executionInfo = defaultExecutionInfo(callback, -1).build();
Review Comment:
I know that it worked this way before, but it seems odd to me that we would
use `-1` for the execution here instead of `callback.execution`. I guess it's
good to leave it this way for consistency like you did, but maybe idea here is
"Something unpredictable unexpected happened here that we can't blame on the
request itself.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
