Sage-Bionetworks / synapsePythonClient

Programmatic interface to Synapse services for Python
https://www.synapse.org
Apache License 2.0
65 stars 67 forks source link

[SYNPY-1356] Refactor sync to synapse #1083

Closed BryanFauble closed 1 month ago

BryanFauble commented 2 months ago

Problem:

  1. The syncToSynapse function provided in the utility package had it's own multi-threaded implementation to allow the upload of files in parallel. Using this it had a shared thread pool executor with the file part upload as well.

Solution:

  1. This PR contains the changes that were independently reviewed in these 2 PRs:
  2. https://github.com/Sage-Bionetworks/synapsePythonClient/pull/1084
  3. https://github.com/Sage-Bionetworks/synapsePythonClient/pull/1089
  4. Conversion of the multi-threaded portion of the syncToSynapse code to instead rely on asyncIO tasks/futures - Meaning all of the code is going to be executed in a single thread and only down the line will parts of files be uploaded in parallel on different threads.

Testing:

  1. Planning to use the benchmark upload script in addition to integration testing around this logic.
pep8speaks commented 2 months ago

Hello @BryanFauble! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 149:89: E501 line too long (89 > 88 characters)

Line 710:89: E501 line too long (89 > 88 characters) Line 1193:89: E501 line too long (91 > 88 characters)

Line 206:89: E501 line too long (117 > 88 characters)

Comment last updated at 2024-05-14 02:00:36 UTC