- 
        Couldn't load subscription status. 
- Fork 577
fix: set changed size when iterating the store's graphs #3281
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
Conversation
| ) -> Generator[_ContextType, None, None]: | ||
| if triple is None or triple == (None, None, None): | ||
| return (context for context in self.__all_contexts) | ||
| return (context for context in list(self.__all_contexts)) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the actual fix.
| default |= c.identifier == DATASET_DEFAULT_GRAPH_ID | ||
| yield c | ||
| if not default: | ||
| yield self.graph(DATASET_DEFAULT_GRAPH_ID) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because the default graph is lazily evaluated, it makes a call here to self.graph, which adds the default graph to the store. This changes the context iterator size and throws a runtime error.
| I'm a bit puzzled as to why this only fails on the 3.8 tests in GitHub Action while it passes fine on python 3.8 locally. | 
| To reproduce the failing test locally, run: ./with-fuseki.sh poetry run pytest -v test/test_dataset/test_dataset.py | 
| The current Python 3.8 tests are extensive and include spinning up a Fuseki instance during the test run. These tests conditionally register the SPARQLUpdateStore plugin only if the Fuseki endpoint is reachable. The issue arises from a change where the Dataset.contexts() method was modified to call the store’s contexts() method directly in 63caed5. However, not all stores guarantee that their contexts() implementation returns graph objects. For example, SPARQLUpdateStore returns graph names as URIRefs instead. To ensure contexts() always returns graph objects, we delegate to ConjunctiveGraph.contexts(), which converts graph name URIRefs into graph objects. | 
Summary of changes
Fix: #3102
The
RuntimeError: Set changed size during iterationoccurs because a Dataset's default graph is lazily evaluated and only added to the store's context dict when accessed.Since the default graph is always present in a dataset, this new change adds the default graph to the context dict on dataset creation.- this change breaks a bunch of tests unrelated to Dataset, like namespace manager etc.We materialise the iterator using a
list()call. This way, when the default graph is added to the contexts dict, we don't get the runtime error.We also avoid calling the super class (ConjunctiveGraph)'s methods as it's redundant and we'll be removing the class hierarchy soon. This now calls
self.store.contextsdirectly.Checklist
the same change.
./examples.so maintainers can fix minor issues and keep your PR up to date.