Java: Add Logger#265
Conversation
jonathanl-bq
requested review from
GumpacG,
Yury-Fridlyand,
aaron-congo,
acarbonetto and
tjzhang-BQ
Yury-Fridlyand
left a comment
Yury-Fridlyand
left a comment
There was a problem hiding this comment.
CI is red.
I think we need tests for this feature.
|
We have few logging TODOs left in client code - can you complete them in scope of this PR? |
Signed-off-by: Yury-Fridlyand <[email protected]>
|
CI hung. You need to rerun spotless though. |
java/client/src/test/java/glide/connection/ConnectionWithGlideMockTests.java
Outdated
Show resolved
Hide resolved
acarbonetto
left a comment
acarbonetto
left a comment
There was a problem hiding this comment.
looks good - a couple of comments
java/client/src/main/java/glide/connectors/handlers/CallbackDispatcher.java
Outdated
Show resolved
Hide resolved
java/client/src/main/java/glide/connectors/handlers/ReadHandler.java
Outdated
Show resolved
Hide resolved
| public final class Logger { | ||
| // TODO: consider lazy loading the glide_rs library | ||
| static { | ||
| System.loadLibrary("glide_rs"); |
There was a problem hiding this comment.
can you move the static loading to the ffi.resolvers package?
Example: https://github.com/Bit-Quill/glide-for-redis/blob/c998a2c366b524270a82c1148bfa477f7ab3318c/java/client/src/main/java/glide/ffi/resolvers/SocketListenerResolver.java
There was a problem hiding this comment.
I think you can remove this static loader - I don't think it doesn't anything here.
Co-authored-by: Andrew Carbonetto <[email protected]>
Co-authored-by: Andrew Carbonetto <[email protected]>
acarbonetto
left a comment
acarbonetto
left a comment
There was a problem hiding this comment.
Remove the extra static loadlibrary please
| public final class Logger { | ||
| // TODO: consider lazy loading the glide_rs library | ||
| static { | ||
| System.loadLibrary("glide_rs"); |
There was a problem hiding this comment.
I think you can remove this static loader - I don't think it doesn't anything here.
4 participants
The Logger implementation here is based on the Python client's implementation, with some relatively small Java/JNI specific changes.