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-1476] Remove thread executor usage for Project, Folder, File #1111

Closed BryanFauble closed 4 days ago

BryanFauble commented 2 weeks ago

Problem:

  1. Within Project, Folder, File, in order to wrap the existing Synapse Python Client .get() function we were running the code within the asyncio run_in_executor. This worked fine up until now. The problem at this point is that utility functions are now going to be directly using these new model functions and it leads to a problem where we are changing from a non-async context (Before SynapseUtils) -> Async context (Inside the model) -> Non-Async context (Running in a thread executor) -> Async context (To download a file) -> Multi-threaded download.
  2. The progress bars for both individual file downloads, and syncFromSynapse was broken and needed to be mended.

Solution:

  1. Breaking out the logic that is used to retrieve entities. This new logic is now following a factory-type pattern allowing us to easily add more entities to it.
  2. Updating the Project, Folder, File, functions to use the new factory-type pattern to retrieve the entity data.
  3. Updating the logic around shared and individual download progress bars

Testing:

  1. Integration test runs
  2. Manual verification via steps laid out in jira comment of: https://sagebionetworks.jira.com/browse/SYNPY-1476
pep8speaks commented 2 weeks ago

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

Line 86:89: E501 line too long (110 > 88 characters) Line 204:89: E501 line too long (91 > 88 characters) Line 205:89: E501 line too long (100 > 88 characters) Line 331:89: E501 line too long (93 > 88 characters) Line 345:89: E501 line too long (93 > 88 characters) Line 365:89: E501 line too long (97 > 88 characters) Line 366:89: E501 line too long (92 > 88 characters) Line 381:89: E501 line too long (95 > 88 characters) Line 382:89: E501 line too long (101 > 88 characters) Line 383:89: E501 line too long (90 > 88 characters)

Line 185:89: E501 line too long (101 > 88 characters) Line 195:89: E501 line too long (101 > 88 characters) Line 196:89: E501 line too long (110 > 88 characters) Line 202:89: E501 line too long (89 > 88 characters) Line 224:89: E501 line too long (104 > 88 characters) Line 247:89: E501 line too long (101 > 88 characters) Line 248:89: E501 line too long (102 > 88 characters)

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

Line 1654:89: E501 line too long (97 > 88 characters) Line 1743:89: E501 line too long (101 > 88 characters) Line 1747:89: E501 line too long (96 > 88 characters)

Comment last updated at 2024-06-24 19:54:16 UTC