Fix clear old sessions for local (BugFix)#1708
Merged
fernando79513 merged 12 commits intomainfrom Feb 19, 2025
Merged
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1708 +/- ##
==========================================
+ Coverage 49.26% 49.30% +0.03%
==========================================
Files 373 373
Lines 40435 40454 +19
Branches 6830 6834 +4
==========================================
+ Hits 19922 19946 +24
+ Misses 19785 19783 -2
+ Partials 728 725 -3
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:
|
Collaborator
fernando79513
left a comment
fernando79513
left a comment
There was a problem hiding this comment.
LGTM!
I tried the --clear-old-session flag and seems to work fine.
Taking a look at the debug logs, they seem really verbose, I wonder if some of them should be traces instead. Anyways, it's not something to tackle here.
fernando79513
previously approved these changes
Feb 10, 2025
Hook25
force-pushed
the
fix_clear_old_sessions
branch
from
4ba6c5f to
3b48ee2
Compare
Minor: this fixes them all, including the debug level
last_job_start_time is mostly None as whenever a job finishes it will be none
Minor: also fix metabox as cmd was overwritten when a manifest was provided
Hook25
force-pushed
the
fix_clear_old_sessions
branch
from
3b48ee2 to
a361693
Compare
* Result fetching for run_cmd * Give the result half a second to materialize * Add result to the ExecuteResult * Preserve all fields on check * Wrong check called * Check is not a property * Trace metabox output * Rollback the binary load to test, make result array * Set the cmd to none in non-interactive for now * Revert trace logging * Purge debug files * Remove pointless append * Use reference in the url comment Co-authored-by: Pierre Equoy <pierre.equoy@canonical.com> --------- Co-authored-by: Pierre Equoy <pierre.equoy@canonical.com>
fernando79513
approved these changes
Feb 19, 2025
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
2 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.
Description
This fixes the
--clear-old-sessionsflag being overall ignored, also, it fixes the way that these flags are used for local only.Lastly, it fixes a missing catch for both local and remote, that would make the agent crash when a selected job is deleted (from an update or manually) and a session is then resumed (as the error won't be InvalidJobError but CorruptedSessionError).
Resolved issues
Fixes: CHECKBOX-1710
Documentation
N/A
Tests
New metabox test that stresses this scenario for local