fix(robot-server, api): Standardize command slice behavior #18203
Conversation
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
mjhuff
changed the title
| # find the most recent command that actually executed, so we need to | ||
| # store that separately. | ||
| cursor = self._state.failed_command.index | ||
| current_pointer = self.get_current() |
There was a problem hiding this comment.
I remember we wanted to do this for a while but we avoided it bc the logic was different? I dont remember why @SyntaxColoring do you remember?
There was a problem hiding this comment.
I don't think it is fundamentally different, at least from my testing and reading, but I've been wrong on at least a couple occasions 😄
There was a problem hiding this comment.
I looked at the implementation again and it does seem correct. I remember wanting to do this change but there was a difference I cannot remember :-) if this was tested then should be fine.
There was a problem hiding this comment.
I think the change I am seeing is in case the last command is a failed command and the queue is empty.
We use the failed_command prop for this case, where with get_current() we loop through the command list to get the last command executed -> they will probably be the same value. also, we are looping though command ids while using get_current and queued commands in the old slice. I think this change is fine though bc the results should be the same.
There was a problem hiding this comment.
Cool cool, thank you for double checking!
There was a problem hiding this comment.
The thing that was always holding me back was the behavioral changes revealed in the test diffs here. It wasn't clear to me that those changes were safe to make, with respect to client expectations. If you've audited the client and it all looks good to you, then yeah, this is great!
TamarZanzouri
approved these changes
Apr 29, 2025
SyntaxColoring
approved these changes
Apr 29, 2025
Comment on lines
+1074
to
+1114
|
|
||
|
|
||
| def test_get_slice_large_length() -> None: | ||
| """It should handle cases where length is larger than available commands.""" | ||
| command_1 = create_succeeded_command(command_id="command-id-1") | ||
| command_2 = create_succeeded_command(command_id="command-id-2") | ||
| command_3 = create_running_command(command_id="command-id-3") | ||
|
|
||
| subject = get_command_view( | ||
| commands=[command_1, command_2, command_3], | ||
| running_command_id="command-id-3", | ||
| ) | ||
|
|
||
| result = subject.get_slice(cursor=None, length=10, include_fixit_commands=True) | ||
|
|
||
| assert result == CommandSlice( | ||
| commands=[command_1, command_2, command_3], | ||
| cursor=0, | ||
| total_length=3, | ||
| ) | ||
|
|
||
|
|
||
| def test_get_slice_explicit_cursor_with_length() -> None: | ||
| """It should use the cursor as the start position when explicitly provided.""" | ||
| command_1 = create_succeeded_command(command_id="command-id-1") | ||
| command_2 = create_succeeded_command(command_id="command-id-2") | ||
| command_3 = create_succeeded_command(command_id="command-id-3") | ||
| command_4 = create_succeeded_command(command_id="command-id-4") | ||
| command_5 = create_succeeded_command(command_id="command-id-5") | ||
|
|
||
| subject = get_command_view( | ||
| commands=[command_1, command_2, command_3, command_4, command_5], | ||
| ) | ||
|
|
||
| result = subject.get_slice(cursor=1, length=3, include_fixit_commands=True) | ||
|
|
||
| assert result == CommandSlice( | ||
| commands=[command_2, command_3, command_4], | ||
| cursor=1, | ||
| total_length=5, | ||
| ) |
There was a problem hiding this comment.
Thank you for the new tests!
| # find the most recent command that actually executed, so we need to | ||
| # store that separately. | ||
| cursor = self._state.failed_command.index | ||
| current_pointer = self.get_current() |
There was a problem hiding this comment.
The thing that was always holding me back was the behavioral changes revealed in the test diffs here. It wasn't clear to me that those changes were safe to make, with respect to client expectations. If you've audited the client and it all looks good to you, then yeah, this is great!
mjhuff
changed the title
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance.
ddcc4
pushed a commit
that referenced
this pull request
May 16, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance.
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 17, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 19, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 19, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 19, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 20, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 20, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 22, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 23, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 24, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
ddcc4
pushed a commit
that referenced
this pull request
May 24, 2025
Closes EXEC-1466 Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way: * When invoking get_slice with identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state. * When a run is historical, an unsupplied cursor but a supplied pageLength returns the most recent pageLength commands, however, when a run is current, only the most recently running/executed command is returned. * The above always applies to run command errors. This commit fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance. (cherry picked from commit 2c374ff)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Closes EXEC-1466
Overview
Currently, when requesting a command slice or a command error slice via the HTTP API, the exact behavior varies depending on the run status, the “current” status of the run, and which parameters are (or are not) supplied. The behavior varies in the following way:
get_slicewith identical cursor and pageLength parameters, there is inconsistency in the number of returned commands when the run is in a recovery state.This PR fixes those issues by standardizing behavior in a manner that is most useful given typical client access patterns: if a cursor is not supplied but a pageLength is supplied, we return the pageLength most recent commands and command errors (so all behavior reflects the current behavior if the command slice is derived from a historical run). After doing a complete client-side audit, the client actually seemingly expects the API to behave this way, and has fortunately worked by chance.
Test Plan and Hands on Testing
Changelog
Risk assessment
lowish? Only the app depends on these resources currently, and it's rather easy to audit/test the spots we care about. Still, this is going into edge for a reason!