Skip to content

Conversation

@mikesmit
Copy link
Collaborator

Related to PolicyEngine/issues#350

This commit adds, but does not yet use, CachingGoogleStorageClient which is a class used to monitor a remote file in google storage for changes caching the result locally to disk.

@mikesmit mikesmit requested a review from anth-volk May 15, 2025 20:09
from policyengine.utils.data import CachingGoogleStorageClient


class MockedStorageSupport:
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion, non-blocking: Separate fixtures out into test/fixtures/data/filepath

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Chatted on slack and decided to put it in a conftest.py file. which I'm doing in a new revision now.

I'm kind of on the fence here. If it's small and only used by one test I tend to want to have it next to the thing being tested, but I also buy it's better to remove clutter and just have the test code in the test file.

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.

Just had one minor suggestion, but otherwise this would be great to have, thanks @mikesmit

@mikesmit mikesmit force-pushed the 350_add_caching_google_storage_client branch 2 times, most recently from 4f3f1b0 to 4e609bf Compare May 15, 2025 21:46
Related to PolicyEngine/issues#350

This commit adds, but does not yet use, CachingGoogleStorageClient which is a class used to monitor a remote file in google storage for changes caching the result locally to disk.
@mikesmit mikesmit force-pushed the 350_add_caching_google_storage_client branch from 4e609bf to 52585de Compare May 15, 2025 21:49
@mikesmit mikesmit merged commit be91f1e into main May 15, 2025
3 checks passed
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