Cleanup transfer manager #619
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
douglas-raillard-arm
force-pushed
the
fix_xfer_mgr
branch
from
5b577e3 to
14e0235
Compare
douglas-raillard-arm
force-pushed
the
fix_xfer_mgr
branch
from
14e0235 to
985bb73
Compare
mrkajetanp
suggested changes
Feb 28, 2023
|
With the above changes the PR makes the PerfettoCollector work fine without specifying an explicit pull timeout so looks like it works :) |
douglas-raillard-arm
force-pushed
the
fix_xfer_mgr
branch
from
985bb73 to
89c8fe4
Compare
|
@mrkajetanp I updated the PR with your review, thanks for testing it |
|
We should be good to go then! I'll update the Collector PR to drop the timeout in that case |
marcbonnici
reviewed
Mar 14, 2023
douglas-raillard-arm
force-pushed
the
fix_xfer_mgr
branch
from
89c8fe4 to
756a96b
Compare
marcbonnici
reviewed
Mar 16, 2023
douglas-raillard-arm
force-pushed
the
fix_xfer_mgr
branch
from
756a96b to
a05d6c3
Compare
|
PR updated with both typos fixed |
marcbonnici
reviewed
Mar 29, 2023
douglas-raillard-arm
force-pushed
the
fix_xfer_mgr
branch
2 times, most recently
from
a053d55 to
46f42fa
Compare
|
PR updated:
Thanks a lot for the testing |
douglas-raillard-arm
force-pushed
the
fix_xfer_mgr
branch
from
46f42fa to
29517b3
Compare
|
PR updated with:
|
marcbonnici
approved these changes
Apr 19, 2023
marcbonnici
requested changes
Apr 29, 2023
Implement a sane interface avoiding variable positional arguments.
douglas-raillard-arm
force-pushed
the
fix_xfer_mgr
branch
from
29517b3 to
492e337
Compare
* Split TransferManager and TransferHandle:
* TransferManager deals with the generic monitoring. To abort a
transfer, it simply cancels the transfer and raises an exception
from manage().
* TransferHandle provides a way for the manager to query the state
of the transfer and cancel it. It is backend-specific.
* Remove most of the state in TransferManager, along with the associated
background command leak etc
* Use a daemonic monitor thread to behave as excpected on interpreter
shutdown.
* Ensure a transfer manager _always_ exists. When no management is
desired, a noop object is used. This avoids using a None sentinel,
which is invariably mishandled by some code leading to crashes.
* Try to merge more paths in the code to uncover as many issues as
possible in testing.
* Fix percentage for SSHTransferHandle (transferred / (remaining +
transferred) instead of transferred / remaining)
* Rename total_timeout TransferManager parameter and attribute to
total_transfer_timeout to match the connection name parameter.
douglas-raillard-arm
force-pushed
the
fix_xfer_mgr
branch
from
492e337 to
5c53ce3
Compare
|
PR updated with s/mgr/manager, and rebased on master (there was a conflict so we might want to give it a last spin before merging) |
marcbonnici
approved these changes
May 5, 2023
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.
Cleanup and simplify transfer manager infrastructure to make it more reliable.
WARNING: Not tested on SSH connection yet.
Tested on android along #618