Build remote registry indices on first use, not at construction - #291
Merged
Conversation
GitHubRegistry and ModuleRemoteRegistry acquired their pooch index in __init__, which reads a cache file or, failing that, fetches one over the network and writes it back. Registries are reconstructed while deserializing an instrument, so constructing one did I/O before anything had asked it for a file. Build the index on first use instead: `pooch` becomes a property that constructs on demand, leaving every existing consumer unchanged. Name, version and filename are interpolated into cache paths and can come from a serialized instrument, so they are now reduced to a single path component; find_registry_file() likewise refuses a name that would resolve outside the package. RemoteRegistry.__eq__ compared pooch internals, which under lazy construction would build an index just to answer an equality test. It now compares the declared name, url, version and filename. That also fixes a pre-existing bug: the old implementation returned False when `other.pooch is None`, so a registry whose index had not been built was not equal to itself, and every equality assertion about one was vacuous.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GitHubRegistry and ModuleRemoteRegistry acquired their pooch index in init, which reads a cache file or, failing that, fetches one over the network and writes it back. Registries are reconstructed while deserializing an instrument, so constructing one did I/O before anything had asked it for a file.
Build the index on first use instead:
poochbecomes a property that constructs on demand, leaving every existing consumer unchanged.Name, version and filename are interpolated into cache paths and can come from a serialized instrument, so they are now reduced to a single path component; find_registry_file() likewise refuses a name that would resolve outside the package.
RemoteRegistry.eq compared pooch internals, which under lazy construction would build an index just to answer an equality test. It now compares the declared name, url, version and filename. That also fixes a pre-existing bug: the old implementation returned False when
other.pooch is None, so a registry whose index had not been built was not equal to itself, and every equality assertion about one was vacuous.