Conversation
…X-1089] Directory upload iterated files sequentially with await _upload_one(). Now uses asyncio.gather with semaphore (max 20 concurrent), matching the pattern in storage/batch.py upload_prefix.
vaibhavatlan
requested review from
OnkarVO7,
atlan-ci and
cmgrote
as code owners
vaibhavatlan
added
the
Autonomous SDK Evolution
vaibhavatlan
added
the
Autonomous SDK Evolution
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Contributor
📜 Docstring Coverage ReportRESULT: PASSED (minimum: 30.0%, actual: 80.0%) Detailed Coverage Report |
Contributor
📦 Trivy Vulnerability Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailsrequirements.txtuv.lock |
Contributor
📦 Trivy Secret Scan Results
Report SummaryCould not generate summary table (data length mismatch: 9 vs 8). Scan Result Detailsrequirements.txtuv.lock |
Collaborator
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified FilesNo covered modified files...
|
Contributor
|
🛠 Full Test Coverage Report: https://k.atlan.dev/coverage/application-sdk/pr/1504 |
vaibhavatlan
changed the title
Collaborator
Author
|
@sdk-review auto-complete |
Contributor
|
🔄 SDK Review starting (auto-complete) — ~10 min, possibly longer with fix iterations. Watch live progress
|
Contributor
SDK Review: PR #1504 — feat(storage): parallelize directory upload in transfer.upload() [BLDX-1089]Verdict: NEEDS FIXES
Findings by File
Strengths
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
- Move `import asyncio` to module top-level (SDK convention) - Replace hardcoded MAX_CONCURRENT_UPLOADS=20 with configurable `max_concurrency: int = 4` kwarg, matching batch.py's API - Add `return_exceptions=True` to asyncio.gather to prevent orphaned tasks on partial failure, re-raising the first error after all tasks settle Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@sdk-review auto-complete |
Contributor
|
🔄 SDK Re-review starting (auto-complete) — ~10 min, possibly longer with fix iterations. Watch live progress
|
Contributor
SDK Re-review: PR #1504 — feat(storage): parallelize directory upload in transfer.upload() [BLDX-1089]Verdict: NEEDS FIXES
Delta from prior review
Findings by File
Strengths
|
Cover the three scenarios requested in review: 1. Multi-file directory upload completing correctly (10 files) 2. transferred_count accuracy with skip_if_exists partial skips 3. Error propagation when one upload raises inside gather Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@sdk-review |
Contributor
|
🔄 SDK Re-review starting (review) — ~10 min. Watch live progress |
Aryamanz29
reviewed
Apr 22, 2026
Member
Aryamanz29
left a comment
Aryamanz29
left a comment
There was a problem hiding this comment.
lets also add integration tests for this
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Cover the concurrent directory upload/download via transfer module against a real local object store: - Multi-file concurrent upload (15 files) - Full upload→download roundtrip with nested directories - Partial skip_if_exists behavior (unchanged vs changed files) - max_concurrency parameter with low concurrency value Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@sdk-review |
Contributor
|
🔄 SDK Re-review starting (review) — ~10 min. Watch live progress |
Contributor
SDK Re-review: PR #1504 — feat(storage): parallelize directory upload in transfer.upload() [BLDX-1089]Verdict: READY TO MERGE
Delta from prior review
Findings by FileNo findings. Strengths
|
Collaborator
|
ℹ️ SDK Review verdict: READY TO MERGE — but required CI checks on this branch are failing. The |
… count list_keys returns sidecar files too, so assert against suffix-filtered keys to match only the 15 data files. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Collaborator
Author
|
@sdk-review |
Contributor
|
🔄 SDK Re-review starting (review) — ~10 min. Watch live progress |
Contributor
SDK Re-review: PR #1504 — feat(storage): parallelize directory upload in transfer.upload() [BLDX-1089]Verdict: READY TO MERGE
Delta from prior review
Findings by FileNo findings. Strengths
|
atlan-ci
previously approved these changes
Apr 22, 2026
Collaborator
atlan-ci
left a comment
atlan-ci
left a comment
There was a problem hiding this comment.
SDK Re-review: no new issues introduced by the latest changes. CI passing. Branch up to date. Approved.
…loads Uses configurable constant (env: ATLAN_MAX_CONCURRENT_STORAGE_TRANSFERS, default 4) instead of hardcoded value, aligning with batch.py convention.
Collaborator
Author
|
@sdk-review |
Contributor
|
🔄 SDK Re-review starting (review) — ~10 min. Watch live progress |
Contributor
SDK Re-review: PR #1504 — feat(storage): parallelize directory upload in transfer.upload() [BLDX-1089]Verdict: READY TO MERGE
Delta from prior review
Findings by FileNo findings. Strengths
|
Member
|
v3 relevance check: ✅ Still relevant — parallelize upload in |
cmgrote
approved these changes
Apr 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What was found
Rule: PERF-016 | File:
application_sdk/storage/transfer.py:254| Severity: HIGHSequential
await _upload_one()in a for loop for directory uploads. No parallelism.What was fixed
Replaced sequential loop with
asyncio.gather+asyncio.Semaphore(20)for bounded concurrent uploads. Matches the pattern instorage/batch.py upload_prefix.Linear
https://linear.app/atlan-epd/issue/BLDX-1089