Skip to content

feat: make GCS artifact service async under the hood#1347

Closed
condorcet wants to merge 3 commits intogoogle:mainfrom
condorcet:async_gcs_artifact_storage
Closed

feat: make GCS artifact service async under the hood#1347
condorcet wants to merge 3 commits intogoogle:mainfrom
condorcet:async_gcs_artifact_storage

Conversation

@condorcet
Copy link
Copy Markdown
Contributor

@condorcet condorcet commented Jun 12, 2025

Fixes issue #1346

DeanChensj
DeanChensj approved these changes Jul 7, 2025 •
edited
Loading
View reviewed changes
@DeanChensj
Copy link
Copy Markdown
Collaborator

DeanChensj commented Jul 7, 2025

Could you resolve the failing checks?

@DeanChensj DeanChensj self-assigned this Jul 7, 2025
@condorcet
Copy link
Copy Markdown
Contributor Author

condorcet commented Jul 11, 2025

Thanks! I will actualize request soon

@condorcet condorcet force-pushed the async_gcs_artifact_storage branch from f5b2321 to af0c5e8 Compare July 16, 2025 09:05
@condorcet
Copy link
Copy Markdown
Contributor Author

condorcet commented Jul 16, 2025

@DeanChensj done

@DeanChensj DeanChensj added ready to pull [Status] This PR is ready to be imported back to Google wip [Status] This issue is being worked on. Either there is a pending PR or is planned to be fixed labels Jul 16, 2025
DeanChensj
DeanChensj requested changes Jul 17, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@DeanChensj DeanChensj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From @Jacksunwei :

asyncio provides a thread-pool-based mechanism for this. We don't need to manage thread pool executor for us.

Let's use asyncio.to_thread approach instead.

@DeanChensj DeanChensj removed the ready to pull [Status] This PR is ready to be imported back to Google label Jul 17, 2025
@condorcet
Copy link
Copy Markdown
Contributor Author

condorcet commented Jul 18, 2025 •
edited
Loading

asyncio provides a thread-pool-based mechanism for this. We don't need to manage thread pool executor for us.

Let's use asyncio.to_thread approach instead.

@DeanChensj @Jacksunwei

In this case we won't be able to configure executor (e.g. number of workers). Under the hood asyncio.to_thread use the same class of ThreadPoolExecutor, but with defaults.

Also asyncio.to_thread copy context vars and this is really seems reasonable.

So, WDYT will it be acceptable for us don't have ability to configure executor?

@Jacksunwei
Copy link
Copy Markdown
Collaborator

Jacksunwei commented Jul 18, 2025

I think it's fine.

@condorcet condorcet force-pushed the async_gcs_artifact_storage branch from 41471d7 to 64d872a Compare July 19, 2025 10:35
@condorcet
Copy link
Copy Markdown
Contributor Author

condorcet commented Jul 19, 2025

@Jacksunwei @DeanChensj done. Now we use asyncio.to_thread instead thread pool executor explicitly

@DeanChensj DeanChensj added the ready to pull [Status] This PR is ready to be imported back to Google label Jul 19, 2025
copybara-service bot pushed a commit that referenced this pull request Jul 21, 2025
@condorcet @copybara-github
feat: make GCS artifact service async under the hood
81e0d40 
Merge #1347

Fixes issue #1346

COPYBARA_INTEGRATE_REVIEW=#1347 from condorcet:async_gcs_artifact_storage 3efee5a
PiperOrigin-RevId: 785488472
@DeanChensj
Copy link
Copy Markdown
Collaborator

DeanChensj commented Jul 22, 2025

This PR is merged in 81e0d40

@DeanChensj DeanChensj closed this Jul 22, 2025
@DeanChensj DeanChensj mentioned this pull request Jul 22, 2025
Closed
pandasanjay pushed a commit to pandasanjay/adk-python that referenced this pull request Jul 28, 2025
@condorcet @copybara-github
feat: make GCS artifact service async under the hood
70b87f8 
Merge google#1347

Fixes issue google#1346

COPYBARA_INTEGRATE_REVIEW=google#1347 from condorcet:async_gcs_artifact_storage 3efee5a
PiperOrigin-RevId: 785488472
knaou pushed a commit to knaou/adk-python that referenced this pull request Aug 16, 2025
@condorcet @knaou
feat: make GCS artifact service async under the hood
d70759a 
Merge google#1347

Fixes issue google#1346

COPYBARA_INTEGRATE_REVIEW=google#1347 from condorcet:async_gcs_artifact_storage 3efee5a
PiperOrigin-RevId: 785488472
@loic-combis loic-combis mentioned this pull request Mar 28, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@DeanChensj DeanChensj DeanChensj approved these changes

Assignees

@DeanChensj DeanChensj

Labels

ready to pull [Status] This PR is ready to be imported back to Google wip [Status] This issue is being worked on. Either there is a pending PR or is planned to be fixed

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@condorcet @DeanChensj @Jacksunwei