Remote assistant refactoring (new) #2057
Merged
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
Hook25
changed the title
Hook25
force-pushed
the
remote_assistant_refactoring
branch
from
da22ab5 to
77156d8
Compare
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## bootstrap_refactor #2057 +/- ##
===================================================
Coverage 51.87% 51.87%
===================================================
Files 387 387
Lines 41684 41697 +13
Branches 7743 7747 +4
===================================================
+ Hits 21623 21630 +7
- Misses 19295 19300 +5
- Partials 766 767 +1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hook25
force-pushed
the
bootstrap_refactor
branch
from
4a19cc2 to
0c78444
Compare
Forgotten removed api
Decorator was moved to top level of the module, state now has a non-private property (that logs transitions) and was changed accordingly Minor fix: use .state and not ._state in decorator
Minor: forgotten 2 state comparison to str instead of new enum
This removes session finalization in all branches. It was added because the start_new_sesison function starts a new session, so it fails on the second iteration. The correct solution to this was not to spam finalize everywhere but to split that function up into start and select_test_plan (thanks me, I know you did this to me)
Minor: use new continue session function
Hook25
force-pushed
the
remote_assistant_refactoring
branch
from
9ba1984 to
77b2beb
Compare
fernando79513
requested changes
Aug 7, 2025
Collaborator
fernando79513
left a comment
fernando79513
left a comment
There was a problem hiding this comment.
Amazing work here!
Overall, everything is clearer than before, and it makes more sense. Thanks for all the docstrings and the renaming.
I didn't find any issues in the code. I went through the issues that you mention in the description, and I agree with the solutions you proposed.
I don't think there are many big changes on the behavior of the sessions after this refactor, but is the information in checkbox remote page still correct?
Co-authored-by: Fernando Bravo <[email protected]>
Collaborator
Author
|
The session behaviour must be exactly the same after this PR. The code I've removed was a legacy resume mechanism that was even more brittle than the one we have right now. |
fernando79513
approved these changes
Aug 8, 2025
Collaborator
fernando79513
left a comment
fernando79513
left a comment
There was a problem hiding this comment.
Yep, that was what I thought, but I wanted to double check it.
I think this is good to go then.
Great work!
Hook25
added a commit
that referenced
this pull request
Aug 8, 2025
* Full refactoring backported to remote_assistant.py * Backport controller refactoring Forgotten removed api * Fix unit tests by renaming apis and changing whats_up response * Refactored allowed_when and _state -> state Decorator was moved to top level of the module, state now has a non-private property (that logs transitions) and was changed accordingly Minor fix: use .state and not ._state in decorator * Renamed resume_session to prepare_resume_session Minor: forgotten 2 state comparison to str instead of new enum * Missing bootstrap_and_continue * Remove outdated resume machanism * Check that all states have a connection_strategy * Support optional parameters of interaction * Include test plan in session metadata * Move functions, rename functions that continue with _continue * Default value of resume_by_id should be empty dict * Propagate prepare_resume_session * Also update name in the test * Don't finalize the session until resume is decided This removes session finalization in all branches. It was added because the start_new_sesison function starts a new session, so it fails on the second iteration. The correct solution to this was not to spam finalize everywhere but to split that function up into start and select_test_plan (thanks me, I know you did this to me) * Port renames and new _continue process to tests * Also fix local changes to subcommands * Test test plan selection function Minor: use new continue session function * Disable fail-fast * Update checkbox-ng/checkbox_ng/launcher/controller.py Co-authored-by: Fernando Bravo <[email protected]> --------- Co-authored-by: Fernando Bravo <[email protected]>
Hook25
added a commit
that referenced
this pull request
Aug 8, 2025
* Rename bootstrap_todo_list to start_bootstrap and use the apis in silent bootstrap Minor: better docstring * Create bootstrapping API to avoid using flags directly * Rename resume_session to prepare_resume_session resume session doesn't really resume the session, more work is needed to get the session fully resumed * Also propagate the bootstrap api change to remote_assistant * Fix unit tests * Test new bootstrapping property * Backward compatible type hints using comments Python3.5 (sigh) doesn't implement pep526 * Bump REMOTE_API_VERSION * Fix typos in the docstrings * Fix outdated todo_list getter * Remote assistant refactoring (new) (#2057) * Full refactoring backported to remote_assistant.py * Backport controller refactoring Forgotten removed api * Fix unit tests by renaming apis and changing whats_up response * Refactored allowed_when and _state -> state Decorator was moved to top level of the module, state now has a non-private property (that logs transitions) and was changed accordingly Minor fix: use .state and not ._state in decorator * Renamed resume_session to prepare_resume_session Minor: forgotten 2 state comparison to str instead of new enum * Missing bootstrap_and_continue * Remove outdated resume machanism * Check that all states have a connection_strategy * Support optional parameters of interaction * Include test plan in session metadata * Move functions, rename functions that continue with _continue * Default value of resume_by_id should be empty dict * Propagate prepare_resume_session * Also update name in the test * Don't finalize the session until resume is decided This removes session finalization in all branches. It was added because the start_new_sesison function starts a new session, so it fails on the second iteration. The correct solution to this was not to spam finalize everywhere but to split that function up into start and select_test_plan (thanks me, I know you did this to me) * Port renames and new _continue process to tests * Also fix local changes to subcommands * Test test plan selection function Minor: use new continue session function * Disable fail-fast * Update checkbox-ng/checkbox_ng/launcher/controller.py Co-authored-by: Fernando Bravo <[email protected]> --------- Co-authored-by: Fernando Bravo <[email protected]> --------- Co-authored-by: Fernando Bravo <[email protected]>
stanley31huang
pushed a commit
that referenced
this pull request
Aug 14, 2025
* Rename bootstrap_todo_list to start_bootstrap and use the apis in silent bootstrap Minor: better docstring * Create bootstrapping API to avoid using flags directly * Rename resume_session to prepare_resume_session resume session doesn't really resume the session, more work is needed to get the session fully resumed * Also propagate the bootstrap api change to remote_assistant * Fix unit tests * Test new bootstrapping property * Backward compatible type hints using comments Python3.5 (sigh) doesn't implement pep526 * Bump REMOTE_API_VERSION * Fix typos in the docstrings * Fix outdated todo_list getter * Remote assistant refactoring (new) (#2057) * Full refactoring backported to remote_assistant.py * Backport controller refactoring Forgotten removed api * Fix unit tests by renaming apis and changing whats_up response * Refactored allowed_when and _state -> state Decorator was moved to top level of the module, state now has a non-private property (that logs transitions) and was changed accordingly Minor fix: use .state and not ._state in decorator * Renamed resume_session to prepare_resume_session Minor: forgotten 2 state comparison to str instead of new enum * Missing bootstrap_and_continue * Remove outdated resume machanism * Check that all states have a connection_strategy * Support optional parameters of interaction * Include test plan in session metadata * Move functions, rename functions that continue with _continue * Default value of resume_by_id should be empty dict * Propagate prepare_resume_session * Also update name in the test * Don't finalize the session until resume is decided This removes session finalization in all branches. It was added because the start_new_sesison function starts a new session, so it fails on the second iteration. The correct solution to this was not to spam finalize everywhere but to split that function up into start and select_test_plan (thanks me, I know you did this to me) * Port renames and new _continue process to tests * Also fix local changes to subcommands * Test test plan selection function Minor: use new continue session function * Disable fail-fast * Update checkbox-ng/checkbox_ng/launcher/controller.py Co-authored-by: Fernando Bravo <[email protected]> --------- Co-authored-by: Fernando Bravo <[email protected]> --------- Co-authored-by: Fernando Bravo <[email protected]>
bladernr
pushed a commit
that referenced
this pull request
Aug 28, 2025
* Rename bootstrap_todo_list to start_bootstrap and use the apis in silent bootstrap Minor: better docstring * Create bootstrapping API to avoid using flags directly * Rename resume_session to prepare_resume_session resume session doesn't really resume the session, more work is needed to get the session fully resumed * Also propagate the bootstrap api change to remote_assistant * Fix unit tests * Test new bootstrapping property * Backward compatible type hints using comments Python3.5 (sigh) doesn't implement pep526 * Bump REMOTE_API_VERSION * Fix typos in the docstrings * Fix outdated todo_list getter * Remote assistant refactoring (new) (#2057) * Full refactoring backported to remote_assistant.py * Backport controller refactoring Forgotten removed api * Fix unit tests by renaming apis and changing whats_up response * Refactored allowed_when and _state -> state Decorator was moved to top level of the module, state now has a non-private property (that logs transitions) and was changed accordingly Minor fix: use .state and not ._state in decorator * Renamed resume_session to prepare_resume_session Minor: forgotten 2 state comparison to str instead of new enum * Missing bootstrap_and_continue * Remove outdated resume machanism * Check that all states have a connection_strategy * Support optional parameters of interaction * Include test plan in session metadata * Move functions, rename functions that continue with _continue * Default value of resume_by_id should be empty dict * Propagate prepare_resume_session * Also update name in the test * Don't finalize the session until resume is decided This removes session finalization in all branches. It was added because the start_new_sesison function starts a new session, so it fails on the second iteration. The correct solution to this was not to spam finalize everywhere but to split that function up into start and select_test_plan (thanks me, I know you did this to me) * Port renames and new _continue process to tests * Also fix local changes to subcommands * Test test plan selection function Minor: use new continue session function * Disable fail-fast * Update checkbox-ng/checkbox_ng/launcher/controller.py Co-authored-by: Fernando Bravo <[email protected]> --------- Co-authored-by: Fernando Bravo <[email protected]> --------- Co-authored-by: Fernando Bravo <[email protected]>
stanley31huang
pushed a commit
that referenced
this pull request
Oct 3, 2025
* Rename bootstrap_todo_list to start_bootstrap and use the apis in silent bootstrap Minor: better docstring * Create bootstrapping API to avoid using flags directly * Rename resume_session to prepare_resume_session resume session doesn't really resume the session, more work is needed to get the session fully resumed * Also propagate the bootstrap api change to remote_assistant * Fix unit tests * Test new bootstrapping property * Backward compatible type hints using comments Python3.5 (sigh) doesn't implement pep526 * Bump REMOTE_API_VERSION * Fix typos in the docstrings * Fix outdated todo_list getter * Remote assistant refactoring (new) (#2057) * Full refactoring backported to remote_assistant.py * Backport controller refactoring Forgotten removed api * Fix unit tests by renaming apis and changing whats_up response * Refactored allowed_when and _state -> state Decorator was moved to top level of the module, state now has a non-private property (that logs transitions) and was changed accordingly Minor fix: use .state and not ._state in decorator * Renamed resume_session to prepare_resume_session Minor: forgotten 2 state comparison to str instead of new enum * Missing bootstrap_and_continue * Remove outdated resume machanism * Check that all states have a connection_strategy * Support optional parameters of interaction * Include test plan in session metadata * Move functions, rename functions that continue with _continue * Default value of resume_by_id should be empty dict * Propagate prepare_resume_session * Also update name in the test * Don't finalize the session until resume is decided This removes session finalization in all branches. It was added because the start_new_sesison function starts a new session, so it fails on the second iteration. The correct solution to this was not to spam finalize everywhere but to split that function up into start and select_test_plan (thanks me, I know you did this to me) * Port renames and new _continue process to tests * Also fix local changes to subcommands * Test test plan selection function Minor: use new continue session function * Disable fail-fast * Update checkbox-ng/checkbox_ng/launcher/controller.py Co-authored-by: Fernando Bravo <[email protected]> --------- Co-authored-by: Fernando Bravo <[email protected]> --------- Co-authored-by: Fernando Bravo <[email protected]>
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.
Description
Currently:
whats_upis poorly documentedThis refactor fixes all the above issues
Resolved issues
Fixes: https://warthogs.atlassian.net/browse/CHECKBOX-1983
Documentation
Added documentation for many APIs
Tests
Metabox tests, nothing should change about the behaviour