Skip to content

Java: Add Logger#1422

Merged
Yury-Fridlyand merged 38 commits intovalkey-io:mainfrom
Bit-Quill:java/integ_lotjonat_logging
Jul 2, 2024
Merged

Java: Add Logger#1422
Yury-Fridlyand merged 38 commits intovalkey-io:mainfrom
Bit-Quill:java/integ_lotjonat_logging

Conversation

@jonathanl-bq
Copy link
Copy Markdown
Contributor

Issue #, if available:
N/A

The Logger implementation here is based on the Python client's implementation, with some relatively small Java/JNI specific changes.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jonathanl-bq jonathanl-bq requested a review from a team as a code owner May 17, 2024 16:44
@Yury-Fridlyand Yury-Fridlyand added the java ☕ issues and fixes related to the java client label May 17, 2024
@jonathanl-bq
Copy link
Copy Markdown
Contributor Author

Panic handling is coming in a separate PR that overhauls how we handle errors in the FFI layer in general.

@Yury-Fridlyand Yury-Fridlyand mentioned this pull request Jun 29, 2024
5 tasks
@barshaul
Copy link
Copy Markdown
Collaborator

@jonathanl-bq Ping me when it's ready

@jduo
Copy link
Copy Markdown
Collaborator

jduo commented Jun 30, 2024

@jonathanl-bq Ping me when it's ready

The panic handling changes got merged already:
#1601

Copy link
Copy Markdown

@Yury-Fridlyand Yury-Fridlyand left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More logging todos were merged today, sorry.
Please see MessageHandler and #1662

@Yury-Fridlyand
Copy link
Copy Markdown

@barshaul round

…dler.java

Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>
@Yury-Fridlyand Yury-Fridlyand dismissed barshaul’s stale review July 1, 2024 22:32

All PR comments were addressed.

Copy link
Copy Markdown
Contributor

@acarbonetto acarbonetto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Getter private static Level loggerLevel;

private static void initLogger(@NonNull Level level, String fileName) {
if (level == Level.DISABLED) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DISABLED option was added to Java side for testing, since we don't want to log when we have mocks. There's other ways to do it (mocking static objects is annoying, but possible), but this is the best way.

@BeforeEach
@SneakyThrows
public void createTestClient() {
Logger.setLoggerConfig(Logger.Level.DISABLED);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right - this is why we need a DISABLED level logger.
Can we add a TODO to add this level to the redis-core side?

@Yury-Fridlyand Yury-Fridlyand merged commit 5cf478d into valkey-io:main Jul 2, 2024
@Yury-Fridlyand Yury-Fridlyand deleted the java/integ_lotjonat_logging branch July 2, 2024 17:26
cyip10 pushed a commit to Bit-Quill/valkey-glide that referenced this pull request Jul 16, 2024
* Implement Logger for Java client

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>
Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>
Co-authored-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Yury-Fridlyand added a commit to Bit-Quill/valkey-glide that referenced this pull request Jun 6, 2025
* Implement Logger for Java client

Signed-off-by: Yury-Fridlyand <yury.fridlyand@improving.com>
Co-authored-by: Yury-Fridlyand <yury.fridlyand@improving.com>
Co-authored-by: Andrew Carbonetto <andrew.carbonetto@improving.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

java ☕ issues and fixes related to the java client

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

5 participants