Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ShellBolt log level #7955

Merged
merged 1 commit into from
Feb 4, 2025
Merged

Conversation

mstrucken
Copy link
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
Contributor Author

Fixes #7954

Copy link
Contributor

@rzo1 rzo1 left a comment

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
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
Copy link
Contributor

rzo1 commented Feb 4, 2025

@mstrucken Most likely: STORM-3961 :)

@reiabreu
Copy link
Contributor

reiabreu commented Feb 4, 2025

Thanks for a detailed issue and PR!

@reiabreu reiabreu merged commit 9566974 into apache:master Feb 4, 2025
12 checks passed
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