Skip to content

Commit 7b9ed4b

Browse files
committed
made max_concurrant more consistent
1 parent 3a30b22 commit 7b9ed4b

1 file changed

Lines changed: 18 additions & 19 deletions

File tree

synapseclient/models/download_list.py

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -338,13 +338,13 @@ def _read_manifest_rows(
338338
return list(columns), rows
339339

340340
@staticmethod
341-
async def _download_manifest_entry(
341+
async def _download_row(
342342
row: dict[str, Any],
343343
download_location: Optional[str] = None,
344344
*,
345345
synapse_client: Optional["Synapse"] = None,
346346
) -> Optional[DownloadListItem]:
347-
"""Download a manifest entry from a manifest row and record the result in place.
347+
"""Download the file described by a manifest row and record the result in place.
348348
349349
On success, sets row["path"] to the local file path and row["error"]
350350
to "". On failure, sets row["path"] to "" and row["error"] to the
@@ -362,6 +362,7 @@ async def _download_manifest_entry(
362362
A DownloadListItem on success, or None on failure.
363363
"""
364364
from synapseclient import Synapse
365+
from synapseclient.models.file import File
365366

366367
client = Synapse.get_client(synapse_client=synapse_client)
367368
entity_id = row["ID"]
@@ -376,12 +377,12 @@ async def _download_manifest_entry(
376377
return None
377378

378379
try:
379-
entity = await client.get_async(
380-
entity_id,
381-
version=version_number,
382-
downloadLocation=download_location,
383-
)
384-
row[_PATH_COLUMN] = entity.path or ""
380+
file = await File(
381+
id=entity_id,
382+
version_number=version_number,
383+
path=download_location,
384+
).get_async(synapse_client=client)
385+
row[_PATH_COLUMN] = file.path or ""
385386
row[_ERROR_COLUMN] = ""
386387
return DownloadListItem(
387388
file_entity_id=entity_id,
@@ -408,7 +409,7 @@ def _write_result_manifest(
408409
path: Destination path for the output manifest CSV.
409410
columns: Field names for the CSV header, including "path" and
410411
"error".
411-
rows: List of row dicts, each mutated by _download_manifest_entry to
412+
rows: List of row dicts, each mutated by _download_row to
412413
include "path" and "error" values.
413414
"""
414415
with open(path, "w", newline="") as f:
@@ -421,24 +422,23 @@ async def _download_all_rows(
421422
rows: list[dict[str, Any]],
422423
download_location: Optional[str],
423424
parallel: bool = False,
424-
max_concurrent: Optional[int] = None,
425+
max_concurrent: int = 10,
425426
*,
426427
synapse_client: Optional["Synapse"] = None,
427428
) -> list[DownloadListItem]:
428429
"""Download all rows from the manifest, either sequentially or concurrently.
429430
430431
Arguments:
431432
rows: List of row dicts from the manifest. Each row is mutated in
432-
place by _download_manifest_entry to include "path" and
433+
place by _download_row to include "path" and
433434
"error" values.
434435
download_location: Directory to download files to.
435436
parallel: If True, rows are downloaded concurrently (bounded by
436437
max_concurrent) via asyncio.gather. If False, rows are
437438
downloaded one at a time.
438439
max_concurrent: Maximum number of concurrent downloads when
439-
parallel=True. Defaults to None, which is treated as
440-
10. Must be at least 1. Has no effect when parallel=False;
441-
a UserWarning is emitted if set explicitly in that case.
440+
parallel=True. Defaults to 10. Must be at least 1. Has no
441+
effect when parallel=False.
442442
synapse_client: Optional Synapse client.
443443
444444
Raises:
@@ -447,11 +447,10 @@ async def _download_all_rows(
447447
Returns:
448448
List of DownloadListItem for each successfully downloaded file.
449449
"""
450-
if max_concurrent is not None and max_concurrent < 1:
450+
if max_concurrent < 1:
451451
raise ValueError(
452452
f"max_concurrent must be at least 1, got {max_concurrent}."
453453
)
454-
max_concurrent = max_concurrent if max_concurrent is not None else 10
455454
if parallel:
456455
# asyncio.gather schedules all coroutines immediately, so without a
457456
# semaphore a large cart would fire hundreds of concurrent HTTP requests
@@ -465,7 +464,7 @@ async def bounded_download(
465464
row: dict[str, Any],
466465
) -> Optional[DownloadListItem]:
467466
async with sem:
468-
return await DownloadList._download_manifest_entry(
467+
return await DownloadList._download_row(
469468
row,
470469
download_location=download_location,
471470
synapse_client=synapse_client,
@@ -476,7 +475,7 @@ async def bounded_download(
476475
else:
477476
downloaded: list[DownloadListItem] = []
478477
for row in rows:
479-
item = await DownloadList._download_manifest_entry(
478+
item = await DownloadList._download_row(
480479
row,
481480
download_location=download_location,
482481
synapse_client=synapse_client,
@@ -494,7 +493,7 @@ async def _save_result_manifest(
494493
"""Write the annotated rows to a new result manifest CSV and return its path.
495494
496495
Arguments:
497-
rows: List of row dicts, each mutated by _download_manifest_entry to
496+
rows: List of row dicts, each mutated by _download_row to
498497
include "path" and "error" values.
499498
columns: Field names for the CSV header, including "path" and
500499
"error".

0 commit comments

Comments
 (0)