Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[FEATURE] Implement TupleFilesystemStoreBackend::get_all #9687

Merged
merged 4 commits into from
Apr 2, 2024
Merged

[FEATURE] Implement TupleFilesystemStoreBackend::get_all #9687

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cdde03d
Add TupleFilesystemStoreBackend::get_all
tyler-hoffman Apr 1, 2024
9db0888
Fix test
tyler-hoffman Apr 2, 2024
c9c58da
Merge branch 'develop' into f/v1-231/TupleFilesystemStoreBackend-get_all
tyler-hoffman Apr 2, 2024
0e717e0
Merge remote-tracking branch 'origin' into f/v1-231/TupleFilesystemSt…
tyler-hoffman Apr 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 2 additions & 1 deletion great_expectations/data_context/store/tuple_store_backend.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,8 @@

@override
def _get_all(self) -> list[Any]:
raise NotImplementedError
keys = [key for key in self.list_keys() if key != StoreBackend.STORE_BACKEND_ID_KEY]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Feels weird to have to exclude this myself. Is there a better option?

Copy link
Member

@Kilo59 Kilo59 Apr 2, 2024 •
edited

Choose a reason for hiding this comment

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

It is getting reported correctly (eventually) but codecov begins commenting/reporting immediately.
https://app.codecov.io/gh/great-expectations/great_expectations/pull/9687/blob/great_expectations/data_context/store/tuple_store_backend.py

image

https://docs.codecov.com/docs/pull-request-comments#after_n_builds

By default Codecov will post and/or update the pull request comment after it processes each report uploaded for a particular pull request commit. If your CI process uploads many reports to codecov, this can be confusing for team members viewing the pull request as reports are processing.

To remedy this, you can delay the posting of a pull request comment until a certain number of reports are received and processed by Codecov, using the after_n_builds setting.

return [self._get(key) for key in keys]

Check warning on line 320 in great_expectations/data_context/store/tuple_store_backend.py

View check run for this annotation

Codecov / codecov/patch

great_expectations/data_context/store/tuple_store_backend.py#L319-L320 

Added lines #L319 - L320 were not covered by tests

def _set(self, key, value, **kwargs):
if not isinstance(key, tuple):
Expand Down
23 changes: 23 additions & 0 deletions tests/data_context/store/test_store_backends.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -476,6 +476,29 @@ def test_TupleFilesystemStoreBackend(tmp_path_factory):
assert url == "http://www.test.com/my_file_CCC"


@pytest.mark.filesystem
def test_TupleFilesystemStoreBackend_get_all(tmp_path_factory):
path = "dummy_str"
full_test_dir = tmp_path_factory.mktemp("test_TupleFilesystemStoreBackend__dir")
project_path = str(full_test_dir)

my_store = TupleFilesystemStoreBackend(
root_directory=project_path,
base_directory=os.path.join(project_path, path), # noqa: PTH118
filepath_template="my_file_{0}",
)

value_a = "aaa"
value_b = "bbb"

my_store.set(("AAA",), value_a)
my_store.set(("BBB",), value_b)

all_values = my_store.get_all()

assert sorted(all_values) == [value_a, value_b]


@pytest.mark.filesystem
def test_TupleFilesystemStoreBackend_ignores_jupyter_notebook_checkpoints(
tmp_path_factory,
Expand Down