-
Notifications
You must be signed in to change notification settings - Fork 18
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.
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
base: dev
Are you sure you want to change the base?
Conversation
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.
Pull request overview
This PR adds support for large addons through sparse checkout of cache. The cache system will perform sparse metadata-only checkouts for extremely large addons, with direct downloads coming from the addon's host rather than the FreeCAD cache.
Changes:
- Added
curatedboolean field to track whether an addon is curated (schema and code) - Added
sparse_cacheboolean field to mark addons that use sparse cache approach (code only) - Modified URL selection logic to force zip_url usage for sparse cache entries
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| AddonCatalog.schema.json | Replaced last_update_time with curated field in schema |
| AddonCatalog.py | Added curated and sparse_cache fields; updated URL selection logic to handle sparse cache entries |
| Addon.py | Added curated field initialization to Addon class |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # 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") |
Copilot
AI
Jan 24, 2026
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.
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.
| # 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." | |
| ) |
| "last_update_time": { | ||
| "type": "string" | ||
| "curated": { | ||
| "type": "boolean" |
Copilot
AI
Jan 24, 2026
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.
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.
| "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." |
| 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") |
Copilot
AI
Jan 24, 2026
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.
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.
There are three addons that currently cannot be installed via the Addon Manager, but have to be installed manually, due to their extremely large size. On the server side a change will soon be made to do a sparse metadata-only checkout, then allowing direct-download of the addons using the Addon Manager, though those downloads will be coming from whatever host the Addon uses, not the FreeCAD-maintained cache.