Add proto changes for pty-shell resizing #2820
Conversation
There was a problem hiding this comment.
Would it be better to have optional uint32 window_rows and optional uint32 window_cols be part of RuntimeInputMessage, and have the worker / runtime construct the magical resize sequence based off this information, rather than the client?
That way, in case we decide to change the magical resize sequence in the future, we don't have to worry about backwards-compatibility with older clients.
Please let me know if you disagree though.
azliu0
force-pushed
the
azliu/runtime-input-resize-proto
branch
from
0f2e4e9 to
296ea32
Compare
There was a problem hiding this comment.
lgtm! @thundergolfer should anyone else review this PR (and the other pty-shell resizing PR's)?
| @@ -2180,6 +2180,8 @@ message RuntimeInputMessage { | |||
| bytes message = 1; | |||
| uint64 message_index = 2; | |||
| bool eof = 3; | |||
| optional uint32 window_rows = 4; | |||
| optional uint32 window_cols = 5; | |||
There was a problem hiding this comment.
- maybe we should comment that window_rows and window_cols are only used for
container exec? - do you think
window_rowsorterminal_rowsorpty_rowsis clearer? (feel free to keep aswindow_rowsif you think that makes more sense to you)
There was a problem hiding this comment.
no strong preference on the naming! willing to defer to anyone else who has a strong opinion on what this field should look like
azliu0
force-pushed
the
azliu/runtime-input-resize-proto
branch
from
296ea32 to
5877963
Compare
Describe your changes
Adds proto changes for handling resize in
pty-shell.Backward/forward compatibility checks
Check these boxes or delete any item (or this section) if not relevant for this PR.
Note on protobuf: protobuf message changes in one place may have impact to
multiple entities (client, server, worker, database). See points above.
Changelog