Skip to content

Support large addons via sparse checkout of cache #329

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.

Sign up for GitHub

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

Closed
chennes wants to merge 1 commit into from
Closed

Support large addons via sparse checkout of cache #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Addon.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,6 +187,7 @@ def __init__(
self.tags = set() # Just a cache, loaded from Metadata
self.remote_last_updated: Optional[datetime.datetime] = None
self.stats = AddonStats()
self.curated = True
self.score = 0

# In cases where there are multiple versions/branches/installations available for an addon,
Expand Down
15 changes: 14 additions & 1 deletion AddonCatalog.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ class AddonCatalogEntry:
branch_display_name: Optional[str] = None
metadata: Optional[CatalogEntryMetadata] = None # Generated by the cache system
last_update_time: str = "" # Generated by the cache system
curated: bool = True # Generated by the cache system
sparse_cache: bool = False # Generated by the cache system
relative_cache_path: str = "" # Generated by the cache system

def __init__(self, raw_data: Dict[str, str]) -> None:
Expand Down Expand Up @@ -135,7 +137,16 @@ def instantiate_addon(self, addon_id: str) -> Addon:
state = Addon.Status.UNCHECKED
else:
state = Addon.Status.NOT_INSTALLED
url = self.repository if self.repository else self.zip_url
if self.sparse_cache:
if self.zip_url:
url = self.zip_url
else:
# TODO: Try to generate the expected URL form based on the repo location
raise RuntimeError(f"Sparse cache entry {addon_id} has no zip_url")
Comment on lines +144 to +145
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The TODO comment suggests generating an expected URL from the repo location when zip_url is missing. However, the code raises a RuntimeError instead. If this fallback behavior is intended, it should be implemented now rather than raising an error. If sparse cache entries are guaranteed to have a zip_url by the server-side cache generation (as suggested by the PR description), then this TODO and the error handling may be unnecessary. Consider either: (1) implementing the URL generation logic now, (2) removing the TODO if the server guarantees zip_url will always be present, or (3) clarifying in the error message that this is a server-side configuration issue.

Suggested change
# TODO: Try to generate the expected URL form based on the repo location
raise RuntimeError(f"Sparse cache entry {addon_id} has no zip_url")
raise RuntimeError(
f"Sparse cache entry '{addon_id}' is missing a 'zip_url'. "
"This indicates a server-side sparse cache configuration or generation error."
)

Copilot uses AI. Check for mistakes.
Comment on lines +140 to +145
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The new sparse_cache functionality lacks test coverage. The instantiate_addon method now has a new code path (lines 140-145) that handles sparse cache entries differently, requiring a zip_url and raising a RuntimeError if it's missing. This new behavior should be tested to ensure it works correctly and that the error message is appropriate. Consider adding tests that cover: (1) sparse_cache=True with valid zip_url, (2) sparse_cache=True without zip_url (should raise RuntimeError), and (3) that the resulting Addon object is correctly configured when using sparse_cache.

Copilot uses AI. Check for mistakes.
elif self.repository:
url = self.repository
else:
url = self.zip_url
if self.git_ref:
addon = Addon(addon_id, url, state, branch=self.git_ref)
else:
Expand Down Expand Up @@ -181,6 +192,8 @@ def instantiate_addon(self, addon_id: str) -> Addon:
self.branch_display_name if self.branch_display_name else self.git_ref
)

addon.curated = self.curated

return addon

@staticmethod
Expand Down
4 changes: 2 additions & 2 deletions AddonCatalog.schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@
"branch_display_name": {
"type": "string"
},
"last_update_time": {
"type": "string"
"curated": {
"type": "boolean"
Copy link

Copilot AI Jan 24, 2026

Choose a reason for hiding this comment

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

The schema replaces last_update_time with curated, but in AddonCatalog.py (line 83), last_update_time is still present and actively used (line 166). This creates a discrepancy. Both fields are marked as "Generated by the cache system" in the Python code. If the schema is validated against cached catalog data, and curated is being added to the schema, then last_update_time should remain in the schema as well, or it should be properly removed from both the schema and the code. Additionally, sparse_cache (line 85 in AddonCatalog.py) is also cache-generated but missing from the schema. With additionalProperties: false set, schema validation will fail if these fields are present in the data but not defined in the schema.

Suggested change
"type": "boolean"
"type": "boolean"
},
"last_update_time": {
"description": "Generated by the cache system: timestamp of the last update for this catalog entry."
},
"sparse_cache": {
"description": "Generated by the cache system: auxiliary cache data associated with this catalog entry."

Copilot uses AI. Check for mistakes.
}
},
"anyOf": [
Expand Down
Loading