yifan-c commented on code in PR #165:
URL:
https://github.com/apache/cassandra-analytics/pull/165#discussion_r2779504070
##########
analytics-sidecar-client/src/main/java/org/apache/cassandra/sidecar/client/StreamBuffer.java:
##########
Review Comment:
We should avoid the changes in the analytics-sidecar-client and the other
sidecar-copied subprojects at the best. Those files are supposed to be in sync
with Sidecar.
I would revert the changes in this file and the test file. The
`VertxStreamBuffer` and its test in another package.
##########
CHANGES.txt:
##########
@@ -1,5 +1,6 @@
0.3.0
-----
+ * Fix ByteBuffer flip() in StreamBuffer.copyBytes() causing data corruption
(CASSANALYTICS-116)
Review Comment:
I believe the description is no longer accurate. Please update to reflect
the actual fix.
##########
analytics-sidecar-client/src/main/java/org/apache/cassandra/sidecar/client/StreamBuffer.java:
##########
@@ -27,11 +27,15 @@
public interface StreamBuffer
{
/**
- * Copies bytes from this {@link StreamBuffer} into the {@link ByteBuffer
destination}
+ * Copies bytes from this {@link StreamBuffer} into the {@link ByteBuffer
destination}.
+ * <p>
+ * This method writes {@code length} bytes starting at the destination
buffer's current position
+ * and advances the position by {@code length}. The caller is responsible
for calling
+ * {@link ByteBuffer#flip()} on the destination buffer before reading from
it.
*
Review Comment:
Thanks for adding the docs. Let's actually add the docs in
`org.apache.cassandra.spark.utils.streaming.StreamBuffer` instead, not this
file.
--
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]
