Skip to content

Add option to set MAX_CONTEXT_URLS in ContextResolver#268

Draft
mielvds wants to merge 1 commit into
masterfrom
104-max-context-urls
Draft

Add option to set MAX_CONTEXT_URLS in ContextResolver#268
mielvds wants to merge 1 commit into
masterfrom
104-max-context-urls

Conversation

@mielvds
Copy link
Copy Markdown
Collaborator

@mielvds mielvds commented May 21, 2026

Fixes #104

This would allow setting a custom ContextResolver with a different level, for instance 20:

contextresolver = ContextResolver(jsonld._resolved_context_cache, options['documentLoader'], max_context_urls=20)
jsonld.compact(input, context, {"contextResolver": contextresolver})

This is a ok fix, but the contextresolver could use a refactor.

  • The cycles parameter in ContextResolver.resolve() is a set(), but can also be an integer.
  • It feels like the default contextresolver should be part of the JsonLdProcessor object instead of being created in the method.
  • according to the docs, "contextResolver" is "for internal use only", but I'm not sure why that is. @davidlehn @dlongley @BigBlueHat ?

@github-actions
Copy link
Copy Markdown

Copy link
Copy Markdown
Collaborator

@anatoly-scherbakov anatoly-scherbakov left a comment

Choose a reason for hiding this comment

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

I believe ContextResolver is said to be internal only because JSON-LD spec does not mandates its existence. It mandates existence and API of DocumentLoader though.

"""

def __init__(self, shared_cache, document_loader):
def __init__(self, shared_cache, document_loader, max_context_urls = None):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
def __init__(self, shared_cache, document_loader, max_context_urls = None):
def __init__(self, shared_cache, document_loader, max_context_urls: int = MAX_CONTEXT_URLS):

self.per_op_cache = {}
self.shared_cache = shared_cache
self.document_loader = document_loader
self._max_context_urls: int = max_context_urls if isinstance(max_context_urls, int) else MAX_CONTEXT_URLS
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Suggested change
self._max_context_urls: int = max_context_urls if isinstance(max_context_urls, int) else MAX_CONTEXT_URLS
self.max_context_urls: int = max_context_urls

self._max_context_urls: int = max_context_urls if isinstance(max_context_urls, int) else MAX_CONTEXT_URLS

@property
def max_context_urls(self) -> int:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Not sure why this is necessary. Perhaps just a property is enough?

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

Successfully merging this pull request may close these issues.

MAX_CONTEXT_URLS set to low

2 participants