Skip to content

Commit

Permalink
adjust timeouts
Browse files Browse the repository at this point in the history
  • Loading branch information
Allie Crevier committed Dec 9, 2019
1 parent 0793d4e commit 033301b
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 14 deletions.
11 changes: 10 additions & 1 deletion securedrop_client/api_jobs/downloads.py
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,10 @@ def call_api(self, api_client: API, session: Session) -> Any:
jobs.
'''

# TODO: Once https://github.com/freedomofpress/securedrop-client/issues/648, we will want to
# pass the default request timeout to download_reply instead of setting it on the api object
# directly.
api_client.default_request_timeout = 20
remote_sources, remote_submissions, remote_replies = \
get_remote_data(api_client)

Expand Down Expand Up @@ -87,7 +91,7 @@ def __init__(self, data_dir: str) -> None:

def _get_realistic_timeout(self, size_in_bytes: int) -> int:
'''
Return a realistic timeout in seconds for a file, message, or reply based on its size.
Return a realistic timeout in seconds based on the size of the download.
This simply scales the timeouts per file so that it increases as the file size increases.
Expand Down Expand Up @@ -260,6 +264,11 @@ def call_download_api(self, api: API, db_object: Reply) -> Tuple[str, str]:
'''
sdk_object = SdkReply(uuid=db_object.uuid, filename=db_object.filename)
sdk_object.source_uuid = db_object.source.uuid

# TODO: Once https://github.com/freedomofpress/securedrop-sdk/issues/108 is implemented, we
# will want to pass the default request timeout to download_reply instead of setting it on
# the api object directly.
api.default_request_timeout = 20
return api.download_reply(sdk_object)

def call_decrypt(self, filepath: str, session: Session = None) -> str:
Expand Down
4 changes: 4 additions & 0 deletions securedrop_client/api_jobs/updatestar.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ def call_api(self, api_client: API, session: Session) -> str:
try:
source_sdk_object = sdclientapi.Source(uuid=self.source_uuid)

# TODO: Once https://github.com/freedomofpress/securedrop-client/issues/648, we will
# want to pass the default request timeout to download_reply instead of setting it on
# the api object directly.
api_client.default_request_timeout = 5
if self.star_status:
api_client.remove_star(source_sdk_object)
else:
Expand Down
5 changes: 5 additions & 0 deletions securedrop_client/api_jobs/uploads.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ def call_api(self, api_client: API, session: Session) -> str:

def _make_call(self, encrypted_reply: str, api_client: API) -> sdclientapi.Reply:
sdk_source = sdclientapi.Source(uuid=self.source_uuid)

# TODO: Once https://github.com/freedomofpress/securedrop-client/issues/648, we will want to
# pass the default request timeout to download_reply instead of setting it on the api object
# directly.
api_client.default_request_timeout = 5
return api_client.reply_source(sdk_source, encrypted_reply, self.reply_uuid)


Expand Down
9 changes: 0 additions & 9 deletions securedrop_client/queue.py
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,6 @@ def logout(self) -> None:

def login(self, api_client: API) -> None:
logger.debug('Passing API token to queues')

# Setting shorter default timeout so that user feedback is faster. For download jobs, this
# timeout is adjusted to be more realistic depending on file size.
#
# Currently ReplyDownloadJobs do not use adjusted timeouts because it is dependent on adding
# an optional timeout parameter to the download_reply securedrop-sdk method, see:
# https://github.com/freedomofpress/securedrop-sdk/issues/108
api_client.default_request_timeout = 5

self.main_queue.api_client = api_client
self.download_file_queue.api_client = api_client
self.start_queues()
Expand Down
27 changes: 23 additions & 4 deletions tests/api_jobs/test_downloads.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ def test_MetadataSyncJob_success(mocker, homedir, session, session_maker):
'securedrop_client.api_jobs.downloads.get_remote_data',
return_value=([mock_source], 'submissions', 'replies'))

api_client = 'foo'
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()

mocker.patch(
'securedrop_client.api_jobs.downloads.update_local_storage',
Expand Down Expand Up @@ -73,7 +75,8 @@ def test_MetadataSyncJob_success_with_key_import_fail(mocker, homedir, session,
'securedrop_client.api_jobs.downloads.get_remote_data',
return_value=([mock_source], 'submissions', 'replies'))

api_client = 'foo'
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()

mocker.patch(
'securedrop_client.api_jobs.downloads.update_local_storage',
Expand Down Expand Up @@ -107,7 +110,8 @@ def test_MetadataSyncJob_success_with_missing_key(mocker, homedir, session, sess
'securedrop_client.api_jobs.downloads.get_remote_data',
return_value=([mock_source], 'submissions', 'replies'))

api_client = 'foo'
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()

mocker.patch(
'securedrop_client.api_jobs.downloads.update_local_storage',
Expand Down Expand Up @@ -149,6 +153,7 @@ def test_ReplyDownloadJob_no_download_or_decrypt(mocker, homedir, session, sessi
mocker.patch.object(job_1.gpg, 'decrypt_submission_or_reply')
mocker.patch.object(job_2.gpg, 'decrypt_submission_or_reply')
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
path = os.path.join(homedir, 'data')
api_client.download_submission = mocker.MagicMock(return_value=('', path))

Expand Down Expand Up @@ -194,6 +199,7 @@ def test_ReplyDownloadJob_message_already_downloaded(mocker, homedir, session, s
job = ReplyDownloadJob(reply.uuid, homedir, gpg)
mocker.patch.object(job.gpg, 'decrypt_submission_or_reply')
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
download_fn = mocker.patch.object(api_client, 'download_reply')

return_uuid = job.call_api(api_client, session)
Expand All @@ -216,6 +222,7 @@ def test_ReplyDownloadJob_happiest_path(mocker, homedir, session, session_maker)
job = ReplyDownloadJob(reply.uuid, homedir, gpg)
mocker.patch.object(job.gpg, 'decrypt_submission_or_reply')
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
data_dir = os.path.join(homedir, 'data')
api_client.download_reply = mocker.MagicMock(return_value=('', data_dir))

Expand Down Expand Up @@ -244,6 +251,7 @@ def test_MessageDownloadJob_no_download_or_decrypt(mocker, homedir, session, ses
mocker.patch.object(job_1.gpg, 'decrypt_submission_or_reply')
mocker.patch.object(job_2.gpg, 'decrypt_submission_or_reply')
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
path = os.path.join(homedir, 'data')
api_client.download_submission = mocker.MagicMock(return_value=('', path))

Expand All @@ -269,6 +277,7 @@ def test_MessageDownloadJob_message_already_decrypted(mocker, homedir, session,
job = MessageDownloadJob(message.uuid, homedir, gpg)
decrypt_fn = mocker.patch.object(job.gpg, 'decrypt_submission_or_reply')
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
download_fn = mocker.patch.object(api_client, 'download_submission')

return_uuid = job.call_api(api_client, session)
Expand All @@ -289,6 +298,7 @@ def test_MessageDownloadJob_message_already_downloaded(mocker, homedir, session,
job = MessageDownloadJob(message.uuid, homedir, gpg)
mocker.patch.object(job.gpg, 'decrypt_submission_or_reply')
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
download_fn = mocker.patch.object(api_client, 'download_submission')

return_uuid = job.call_api(api_client, session)
Expand All @@ -311,6 +321,7 @@ def test_MessageDownloadJob_happiest_path(mocker, homedir, session, session_make
job = MessageDownloadJob(message.uuid, homedir, gpg)
mocker.patch.object(job.gpg, 'decrypt_submission_or_reply')
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
data_dir = os.path.join(homedir, 'data')
api_client.download_submission = mocker.MagicMock(return_value=('', data_dir))

Expand All @@ -332,6 +343,7 @@ def test_MessageDownloadJob_with_base_error(mocker, homedir, session, session_ma
gpg = GpgHelper(homedir, session_maker, is_qubes=False)
job = MessageDownloadJob(message.uuid, homedir, gpg)
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
mocker.patch.object(api_client, 'download_submission', side_effect=BaseError)
decrypt_fn = mocker.patch.object(job.gpg, 'decrypt_submission_or_reply')

Expand All @@ -357,7 +369,7 @@ def test_MessageDownloadJob_with_crypto_error(mocker, homedir, session, session_
job = MessageDownloadJob(message.uuid, homedir, gpg)
mocker.patch.object(job.gpg, 'decrypt_submission_or_reply', side_effect=CryptoError)
api_client = mocker.MagicMock()
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
path = os.path.join(homedir, 'data')
api_client.download_submission = mocker.MagicMock(return_value=('', path))

Expand All @@ -380,6 +392,7 @@ def test_FileDownloadJob_message_already_decrypted(mocker, homedir, session, ses
job = FileDownloadJob(file_.uuid, homedir, gpg)
decrypt_fn = mocker.patch.object(job.gpg, 'decrypt_submission_or_reply')
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
download_fn = mocker.patch.object(api_client, 'download_submission')

return_uuid = job.call_api(api_client, session)
Expand All @@ -400,6 +413,7 @@ def test_FileDownloadJob_message_already_downloaded(mocker, homedir, session, se
job = FileDownloadJob(file_.uuid, os.path.join(homedir, 'data'), gpg)
patch_decrypt(mocker, homedir, gpg, file_.filename)
api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
download_fn = mocker.patch.object(api_client, 'download_submission')

return_uuid = job.call_api(api_client, session)
Expand Down Expand Up @@ -429,6 +443,7 @@ def fake_download(sdk_obj: SdkSubmission, timeout: int) -> Tuple[str, str]:
return ('', full_path)

api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
api_client.download_submission = fake_download

job = FileDownloadJob(
Expand Down Expand Up @@ -471,6 +486,7 @@ def fake_download(sdk_obj: SdkSubmission, timeout: int) -> Tuple[str, str]:
full_path)

api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
api_client.download_submission = fake_download

job = FileDownloadJob(
Expand Down Expand Up @@ -506,6 +522,7 @@ def fake_download(sdk_obj: SdkSubmission, timeout: int) -> Tuple[str, str]:
full_path)

api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
api_client.download_submission = fake_download

job = FileDownloadJob(
Expand Down Expand Up @@ -538,6 +555,7 @@ def fake_download(sdk_obj: SdkSubmission, timeout: int) -> Tuple[str, str]:
full_path)

api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
api_client.download_submission = fake_download

job = FileDownloadJob(
Expand Down Expand Up @@ -581,6 +599,7 @@ def fake_download(sdk_obj: SdkSubmission, timeout: int) -> Tuple[str, str]:
full_path)

api_client = mocker.MagicMock()
api_client.default_request_timeout = mocker.MagicMock()
api_client.download_submission = fake_download

job = FileDownloadJob(
Expand Down

0 comments on commit 033301b

Please sign in to comment.