-
Notifications
You must be signed in to change notification settings - Fork 1.2k
src: Add compact JSON to Printer #22697
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
base: main
Are you sure you want to change the base?
Conversation
13f534c to
caed9bb
Compare
jelly
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you go to the effort add pretty printing why not add a flag for it for the cli? I assume you want to pretty print:
PYTHONPATH=src python3 -m cockpit.misc.print open echo channel=x
And you can't use jq because it emits Cockpit protocol frames.
| """Send a json message (built from **kwargs) on a channel""" | ||
| self.data(channel, json.dumps(kwargs, indent=2).encode() + b'\n') | ||
| # Cockpit expect JSON values to be compact, otherwise it won't parse it correctly | ||
| params: Any = {"separators": None, "indent": None} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick/hint, we have JsonObject in Python
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't pass local checks due to params["key"] assignment below
Argument of type "tuple[Literal[','], Literal[':']]" cannot be assigned to parameter "value" of type "None" in function "__setitem__"
"tuple[Literal[','], Literal[':']]" is not assignable to "None"PylancereportArgumentType
src/cockpit/misc/print.py
Outdated
| ) -> None: | ||
| """Send a json message (built from **kwargs) on a channel""" | ||
| self.data(channel, json.dumps(kwargs, indent=2).encode() + b'\n') | ||
| # Cockpit expect JSON values to be compact, otherwise it won't parse it correctly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is that true?
PYTHONPATH=src python3 -m cockpit.misc.print open echo channel=x : data x "b'foo'" : done x | PYTHONPATH=src python3 -m cockpit.bridge
Works fine with indent is 40, we also don't set separators by default so they are (',', ': '). So you can simplify this by just making it pretty print more indented but that doesn't seem to change too much
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is true when it comes to authorize specifically
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Improved the comment a bit
By default the printer now outputs compact JSON which is more aligned with what is expected of Cockpit. When using `control` it will now print it compact by default. This can be changed by using `--pretty-json` argument. Signed-off-by: Freya Gustavsson <[email protected]>
caed9bb to
270ad58
Compare
|
Added 📦[spytec@cockpit]~/projects/cockpit/cockpit% PYTHONPATH=src python3 -m cockpit.misc.print control authorize cookie="session26151766064575" response="Basic YWRtaW46Zm9vYmFy"
51
{"command":"init","host":"localhost","version":1}
94
{"command":"authorize","cookie":"session26151766064575","response":"Basic YWRtaW46Zm9vYmFy"}
📦[spytec@cockpit]~/projects/cockpit/cockpit% PYTHONPATH=src python3 -m cockpit.misc.print --pretty-json control authorize cookie="session26151766064575" response="Basic YWRtaW46Zm9vYmFy"
70
{
"command": "init",
"host": "localhost",
"version": 1
}
113
{
"command": "authorize",
"cookie": "session26151766064575",
"response": "Basic YWRtaW46Zm9vYmFy"
} |
By default the printer now outputs compact JSON which is more aligned
with what is expected of Cockpit. When using
controlit will now printit compact by default.
Signed-off-by: Freya Gustavsson [email protected]