feat(cli): add ability for user to cancle file transfer in CLI via CTRL+C#2341
feat(cli): add ability for user to cancle file transfer in CLI via CTRL+C#2341LevinCeglie wants to merge 8 commits into
Conversation
c003810 to
602335d
Compare
Greptile SummaryThis PR adds CLI cancellation support for file transfers. The main changes are:
Confidence Score: 5/5This looks safe to merge.
|
| Filename | Overview |
|---|---|
| cli/kleinkram/api/file_transfer.py | Adds transfer cancellation state, worker cleanup, retry interruption, canceled result states, and partial-file cleanup. |
| cli/kleinkram/cli/_download.py | Adds canceled download counts to summaries and exits with code 130 after cancellation. |
| cli/kleinkram/cli/_upload.py | Adds canceled upload counts to summaries and exits with code 130 after cancellation. |
| cli/kleinkram/errors.py | Adds a cancellation marker error for transfer workers. |
Reviews (5): Last reviewed commit: "fix(cli): fix data deletion of cleanup d..." | Re-trigger Greptile
| except KeyboardInterrupt: | ||
| logger.info("Upload interrupted by user, cancelling...") | ||
| interrupted = True | ||
| cancel_event.set() |
There was a problem hiding this comment.
Running workers still block
shutdown(wait=False, cancel_futures=True) only cancels work that has not started. Any worker already inside the S3 or HTTP transfer continues running on a non-daemon executor thread, which keeps the Python process alive after the summary is printed and exit code 130 is raised. A Ctrl+C can still appear to hang until those in-flight transfer calls return.
Prompt To Fix With AI
This is a comment left during a code review.
Path: cli/kleinkram/api/file_transfer.py
Line: 681
Comment:
**Running workers still block** `shutdown(wait=False, cancel_futures=True)` only cancels work that has not started. Any worker already inside the S3 or HTTP transfer continues running on a non-daemon executor thread, which keeps the Python process alive after the summary is printed and exit code 130 is raised. A Ctrl+C can still appear to hang until those in-flight transfer calls return.
How can I resolve this? If you propose a fix, please make it concise.There was a problem hiding this comment.
not an issue, cancel_event is checked every chunk during download and during each boto3_cb callback during upload.
There was a problem hiding this comment.
That's a fair point — if cancel_event is checked on every chunk (download) and every boto3_cb callback (upload), in-flight workers will exit their inner loops promptly after the event is set. That does effectively address the blocking concern; shutdown(wait=False) just avoids waiting for the already-cooperating threads to finish their current loop iteration.
I'll withdraw the concern. Thanks for the clarification.
Tip: You can customize Greptile's behavior for this repo with .greptile/rules.md and .greptile/config.json.
6ad4311 to
c7ff0a9
Compare
c7ff0a9 to
9e4dfaf
Compare
This PR implements robust cancelation logic for file transfers via the CLI. With these changes, the user can interrupt file uploads and downloads using CTRL+C (KeyboardInterrupt) and the CLI will gracefully cancel the transfers and still create a summary.