-
Notifications
You must be signed in to change notification settings - Fork 436
run long requests on second thread #1212
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
Conversation
added a thread for the request, so we can continue giving progress updates on the UI added tests - could do with more
Also begins extracting constants for user editing
Configure it centrally. Still needs work to let the calling user configure it.
Note for review: this does contain 78 files, but |
@@ -262,7 +271,11 @@ def publish( | |||
|
|||
# Determine if chunking is required (64MB is the limit for single upload method) | |||
if file_size >= FILESIZE_LIMIT: | |||
logger.info("Publishing {0} to server with chunking method (datasource over 64MB)".format(filename)) | |||
logger.info( | |||
"Publishing {} to server with chunking method (datasource over {}MB, chunk size {}MB)".format( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are FILESIZE_LIMIT
and CHUNK_SIZE
in bytes, not MB?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops, yea
logger.debug("{} Waiting....".format(datetime.timestamp())) | ||
if minutes % 5 == 0 and seconds >= 60: | ||
logger.info("[{}] Waiting ({} minutes so far) for request to {}".format(datetime.timestamp(), minutes, url)) | ||
elif minutes % 1 == 0 and seconds >= 60: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this supposed to be minutes == 1
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no, originally I was just logging every minute and then I added the longer message every 5 minutes and borked the checks
c2af35f
to
cbba950
Compare
* run long requests on second thread * improve chunked upload requests * begin extracting constants for user editing * centrally configured logger
added a thread for the request, so we can continue giving progress updates on the UI while downloading.
added tests - could do with more
Was not able to repro the original issue (ran into other problems with a large file) so this may not help, but it should at least capture more logs of the failure.