fix: Decode JSON directly from stream in waitForCommandOutput#28040
Merged
codingllama merged 1 commit intomasterfrom Jun 20, 2023
Merged
fix: Decode JSON directly from stream in waitForCommandOutput#28040codingllama merged 1 commit intomasterfrom
waitForCommandOutput#28040codingllama merged 1 commit intomasterfrom
Conversation
greedy52
approved these changes
Jun 19, 2023
camscale
approved these changes
Jun 20, 2023
strideynet
approved these changes
Jun 20, 2023
public-teleport-github-review-bot
Bot
removed request for
gzdunek,
jakule and
timothyb89
jakule
pushed a commit
that referenced
this pull request
Jul 14, 2023
github-merge-queue Bot
pushed a commit
that referenced
this pull request
Jul 14, 2023
* [Assist] Clear the refresh websocket timeout when closing Assist (#28610) * Clear the refresh websocket timeout when closing Assist * Missing semicolon to please prettier * Add comment to remove once the new session implementation is done * fix: Decode JSON directly from stream in `waitForCommandOutput` (#28040) * Assist: mark individual nodes as finished (#28477) * Mark nodes as done when command finishes in Assist * Split Close/CloseWithPayload * Expect a close message before the summary in test * Remediate confusing type usage in test `Envelope` is the outer layer used (in protobuf format) for execution / terminal sessions. Meanwhile `outEnvelope` is the inner layer used (in JSON) spefically with assist (execution), when outer envelope is of "raw" type Using `Envelope` where decoding `outEnvelope` in tests previously worked "by accident" due to matching field names * Assert that server ID is set in the close message * Refactor command execution logic and adjust WebSocket handling This commit changes how command execution and WebSocket handling are performed in the code. Instead of manually managing session close signals and command execution notifications via WebSocket, we have wrapped it into more easily manageable form. Changed parts: assist and websocket libraries, test and several components of 'teleport'. Why: This allows cleaner command management and error handling, leading to more reliable and maintainable application. Also made code more readable and easy to understand. Made WebSocket more precise with its handling to prevent cases where stale or incorrect data might interrupt the session. Details: Instead of sending a close signal after all commands have been executed, we now send individual session end updates for each command. Sessions no longer remain active unnecessarily. Message handling in command execution has been refactored for better error propagation. All these changes were also adjusted and reflected in the associated test cases. Also, names and types of various data structures are edited to reflect their actual usage. * Change ServerID to NodeID to improve code consistency This commit replaces occurrences of `ServerID` with `NodeID` across several files. `ServerID` was misleading and causing confusion as it was serving as an identifier for nodes, so to improve comprehension and consistency in the codebase, all instances of `ServerID` have been replaced with `NodeID`. Tests have also been updated to reflect this change. --------- Co-authored-by: Jakub Nyckowski <jakub.nyckowski@goteleport.com> --------- Co-authored-by: Ryan Clark <ryan.clark@goteleport.com> Co-authored-by: Alan Parra <alan.parra@goteleport.com> Co-authored-by: Justinas Stankevičius <justinas@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes local breakage of
TestExecuteCommandHistory.Without the fix I consistently get this failure:
While debugging I noticed that the 100 bytes buffer in
waitForCommandOutputis too small, so instead of increasing the buffer I removed it altogether.