Skip to content

Fix ShellBolt log level#7955

Merged
reiabreu merged 1 commit into
apache:masterfrom
mstrucken:fix-shellbolt-log-level
Feb 4, 2025
Merged

Fix ShellBolt log level#7955
reiabreu merged 1 commit into
apache:masterfrom
mstrucken:fix-shellbolt-log-level

Conversation

@mstrucken

Copy link
Copy Markdown
Contributor

What is the purpose of the change

ShellBolt log level is always INFO due to failed check against Long after deseralizing

How was the change tested

I used the following minimal example:

String json = "{\"command\": \"log\", \"msg\": \"Testing ...\", \"level\": 4}";

try {
    JSONObject msg = (JSONObject) JSONValue.parseWithException(json);

    ShellMsg shellMsg = new ShellMsg();
    String command = (String)msg.get("command");
    if (command.equals("log")) {
        Object logLevelObj = msg.get("level");
        if (logLevelObj != null && logLevelObj instanceof Number) {
            int logLevel = ((Number) logLevelObj).intValue();
            shellMsg.setLogLevel(logLevel);
        }
    }
} catch (ParseException e) {
    throw new RuntimeException(e);
}

@mstrucken

Copy link
Copy Markdown
Contributor Author

Fixes #7954

@rzo1 rzo1 left a comment

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.

Thanks for the PR - very much appreciated! This is a regression from the switch to another json impl in the last year.

@rzo1 rzo1 added the bug label Feb 4, 2025
@rzo1 rzo1 added this to the 2.8.1 milestone Feb 4, 2025
@mstrucken

Copy link
Copy Markdown
Contributor Author

Thanks for the PR - very much appreciated! This is a regression from the switch to another json impl in the last year.

A colleague told me that it was introduced with 2.6.0, but I haven't found the exact commit.

@rzo1

rzo1 commented Feb 4, 2025

Copy link
Copy Markdown
Contributor

@mstrucken Most likely: STORM-3961 :)

@reiabreu

reiabreu commented Feb 4, 2025

Copy link
Copy Markdown
Contributor

Thanks for a detailed issue and PR!

@reiabreu reiabreu merged commit 9566974 into apache:master Feb 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants