Conversation
| #[derive(Debug, Serialize, Deserialize, Clone)] | ||
| pub struct CommInfoRequest { | ||
| pub target_name: String, | ||
| #[serde(default)] |
There was a problem hiding this comment.
I noticed that https://github.com/posit-dev/ark/pull/1073/changes#diff-f6c818169bdba550162747979f6b260e31e570d6ed62dc25d2d0a203aa01d114 was also adding default everywhere. What's up with that?
There was a problem hiding this comment.
This is needed to make the field truly optional, so it can be absent from the json.
In the other PR, even though the fields are technically required within each variant type, the protocol also notes:
Most of the history messaging options are not used by Jupyter frontends, and many kernels do not implement them. If you’re implementing these messages in a kernel, the ‘tail’ request is the most useful; this is used by the Qt console, for example. The notebook interface does not use history messages at all.
2 participants
Progress towards #1069.
The
target_namefield ofcomm_info_requestis optional in the Jupyter protocol. When absent, the server should provide info for all comms.Currently we're treating
""as the all comm sentinel, but the protocol really specifies the field as optional (can be absent). To fix this, the field is now anOption<String>instead of aString.