Skip to content

Conversation

@mikesmit
Copy link
Collaborator

Related to PolicyEngine/issues#350

this change re-implements the download_file_from_gcs function to use CachingGoogleStorageClient and removes the async stuff from the client to avoid issues with the existing event loop.

Files should now be cached locally on a per-process basis and only updated on disk when the remote version crc changes.

@mikesmit mikesmit force-pushed the 350_use_caching_no_async branch 2 times, most recently from 253e1ea to b6ac808 Compare May 19, 2025 20:41
@mikesmit mikesmit marked this pull request as ready for review May 19, 2025 20:45
@mikesmit mikesmit requested a review from anth-volk May 19, 2025 21:36
@@ -1,0 +1,5 @@
=======
Copy link
Contributor

Choose a reason for hiding this comment

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

issue, minor, blocking: Please remove this line

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

oops. Will do.

Related to PolicyEngine/issues#350

this change re-implements the download_file_from_gcs function to use CachingGoogleStorageClient and removes the async stuff from the client to avoid issues with the existing event loop.

Files should now be cached locally on a per-process basis and only updated on disk when the remote version crc changes.
@mikesmit mikesmit force-pushed the 350_use_caching_no_async branch from b6ac808 to 646cfe0 Compare May 19, 2025 22:25
Copy link
Contributor

@anth-volk anth-volk left a comment

Choose a reason for hiding this comment

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

@mikesmit Left one minor blocking comment that I believe you already resolved. Otherwise, looks good to me!

@mikesmit mikesmit merged commit 5252f6f into main May 19, 2025
4 checks passed
@nikhilwoodruff nikhilwoodruff deleted the 350_use_caching_no_async branch May 20, 2025 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants