Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

app: move export calls to separate thread #563

Merged
merged 3 commits into from
Oct 17, 2019
Merged

Conversation

redshiftzero
Copy link
Contributor

@redshiftzero redshiftzero commented Oct 10, 2019

Description

Fixes #561

Test Plan

Also for testing I've added some debug lines (which are worth leaving in) to demonstrate these export actions are running in the right thread (export_thread), logs from a test run just now:

2019-10-10 11:14:24,538 - securedrop_client.api_jobs.downloads:130(_download) INFO: File downloaded: 2-somber_junction-doc.gz.gpg
2019-10-10 11:14:24,588 - securedrop_client.api_jobs.downloads:147(_decrypt) INFO: File decrypted: 2-somber_junction-doc.gz.gpg
2019-10-10 11:14:24,980 - securedrop_client.logic:617(run_export_preflight_checks) DEBUG: Calling export preflight checks from thread 140735889998720
2019-10-10 11:14:24,981 - securedrop_client.export:237(run_preflight_checks) DEBUG: beginning preflight checks in thread 123145349275648
2019-10-10 11:14:29,982 - securedrop_client.export:239(run_preflight_checks) DEBUG: completed preflight checks: success
2019-10-10 11:14:29,983 - securedrop_client.gui.widgets:2000(_request_passphrase) DEBUG: requesting passphrase...
2019-10-10 11:14:31,129 - securedrop_client.logic:636(export_file_to_usb_drive) DEBUG: Exporting 3-banned_sinker-msg from thread 140735889998720
2019-10-10 11:14:31,130 - securedrop_client.export:258(send_file_to_usb_device) DEBUG: beginning export from thread 123145349275648
2019-10-10 11:14:36,134 - securedrop_client.export:262(send_file_to_usb_device) DEBUG: Export successful

(GUI thread is 140735889998720, export thread is 123145349275648)

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

  • I have tested these changes in the appropriate Qubes environment - what I've tested here is the USB_NO_SUPPORTED_ENCRYPTION case
  • I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
  • These changes should not need testing in Qubes

@redshiftzero
Copy link
Contributor Author

CI failures here appear to be due to partially degraded service for circle CI docker builds, will rerun in a bit (it's happening also on master)

@redshiftzero redshiftzero force-pushed the export-thread branch 2 times, most recently from 5566656 to a83e2c9 Compare October 15, 2019 22:27
@redshiftzero
Copy link
Contributor Author

redshiftzero commented Oct 15, 2019

some changes here relevant to #557 (a cancel mechanism for export) and #558 (5 minute timeout for export): after this last commit on this branch, since we haven't yet implemented the cancel mechanisms (i.e. if a user clicks off the exportdialog, the window doesn't close), if a user starts an export, then the export dialog will stay up and block input to the parent window until it either succeeds or fails. So the cancel mechanisms are strong candidates to do soon as this will be confusing in a demo setting.

@redshiftzero redshiftzero changed the title [wip] app: move export calls to separate thread app: move export calls to separate thread Oct 16, 2019
@redshiftzero redshiftzero marked this pull request as ready for review October 16, 2019 21:20
@redshiftzero
Copy link
Contributor Author

OK played with this a bit in qubes today - ready for more eyes!

@sssoleileraaa sssoleileraaa self-assigned this Oct 17, 2019
@sssoleileraaa
Copy link
Contributor

Nice work! I did some testing and see how the UI is no longer blocked during cross-vm operations. This means that a user can click outside of the dialog to close it during the vm-boot-up and file-transfer stages. We should continue discussion in #557 and in the next client meeting. As far as this PR goes, all looks good to me.

@sssoleileraaa sssoleileraaa self-requested a review October 17, 2019 23:52
@sssoleileraaa sssoleileraaa merged commit f6cb83c into master Oct 17, 2019
@sssoleileraaa sssoleileraaa deleted the export-thread branch October 17, 2019 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

move export calls to their own QThread
2 participants