-
Notifications
You must be signed in to change notification settings - Fork 893
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.
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
ShareMemoryDataset does not have exists()
method
#3703
Comments
The exists() method is not an abstract method and so doesn't have to be implemented. The original |
@merelcht Haven't look into this in details, would the alternative be changing that WARNING message to |
Hi, I investigated this for a bit and I believe the issue is that kedro/kedro/io/shared_memory_dataset.py Lines 33 to 37 in 56e56a7
However, We could implement class SharedMemoryDataset(AbstractDataset):
...
def _exists(self) -> bool:
if not self.shared_memory_dataset:
return False
return self.shared_memory_dataset.exists() I'm happy to submit a PR implementing this:) |
Description
Context
Unclear, the
SharedMemoryDataset
is implemented by the core team and generating warning when run withParallelRunner
, is this false alarm or something we need to address? Cc @merelchtSteps to Reproduce
I was testing something else and see the warning with this command:
pytest tests/framework/session/test_session_extension_hooks.py::TestNodeHooks::test_on_node_error_hook_parallel_runner
Possible Solution:
https://github.com/kedro-org/kedro/blob/0fc8089b637a0679f71e2bddc91f0676fc2914a2/kedro/io/memory_dataset.py#L74C1-L75C40
We have a simple check in
MemoryDataset
, we could implement simliar logic forShareMemoryDataset
.Check whether it makes sense to have an
_exists()
method forSharedMemoryDataset
. If not: update the logging level to be ofDEBUG
.The text was updated successfully, but these errors were encountered: