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.

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

Do we need a global RESOLVER_FOR_URI_SCHEME? #600

Open
lukpueh opened this issue May 15, 2023 · 0 comments
Open

Do we need a global RESOLVER_FOR_URI_SCHEME? #600

lukpueh opened this issue May 15, 2023 · 0 comments

Comments

@lukpueh
Copy link
Member

lukpueh commented May 15, 2023

The ITE-4 resolver interface provides a method to return the right resolver for a given artifact uri, e.g.:

Resolver.for_uri("file:path/to/foo") returns the FileResolver instance that was previously registered in the global RESOLVER_FOR_URI_SCHEME dictionary under the "file" scheme.

This design is above all useful to separate resolver configuration from usage, and to implement uri parsing and default dispatching behind the interface.

However, given that we currently configure and use resolvers in the same function, the design seems more complex and intransparent than useful:

in-toto/in_toto/runlib.py

Lines 162 to 192 in 4581e4b

# Configure resolver with resolver specific arguments
# FIXME: This should happen closer to the user boundary, where
# resolver-specific config arguments are passed and global state is managed.
RESOLVER_FOR_URI_SCHEME[FileResolver.SCHEME] = FileResolver(
exclude_patterns,
base_path,
follow_symlink_dirs,
normalize_line_endings,
lstrip_paths,
)
# Configure resolver for OSTree
RESOLVER_FOR_URI_SCHEME[OSTreeResolver.SCHEME] = OSTreeResolver(base_path)
# Aggregate artifacts per resolver
resolver_for_uris = defaultdict(list)
for artifact in artifacts:
resolver = Resolver.for_uri(artifact)
resolver_for_uris[resolver].append(artifact)
# Hash artifacts in a batch per resolver
# FIXME: The behavior may change if we hash each artifact individually,
# because the left-prefix duplicate check in FileResolver only works for the
# artifacts hashed in one batch.
for resolver, uris in resolver_for_uris.items():
artifact_hashes.update(resolver.hash_artifacts(uris))
# Clear resolvers to not preserve global state change beyond this function.
# FIXME: This also clears resolver registered elsewhere. For now we
# assume that we only modify RESOLVER_FOR_URI_SCHEME in this function.
RESOLVER_FOR_URI_SCHEME.clear()

Unless we detach resolver configuration from resolver usage, I think, it would be easier to keep a local resolver map and also dispatch locally.

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

No branches or pull requests

1 participant