Persist content pack choice #907
Conversation
manuq
force-pushed
the
T35159-persist-pack-selection
branch
3 times, most recently
from
f7eb3cc to
51a5d67
Compare
kolibri_explore_plugin/models.py
Outdated
|
|
||
| name = models.SlugField( | ||
| blank=False, | ||
| null=False, |
There was a problem hiding this comment.
Adding null=False is somewhat redundant here: that's the default, and it would only kick in if blank is True anyway ;)
| def ensure_initiated(api_function): | ||
| """Decorator to initiate only once in the first API call.""" | ||
|
|
||
| def wrapper(*args, **kwargs): | ||
| if _collection_download_manager is None: | ||
| _read_content_manifests() | ||
| return api_function(*args, **kwargs) | ||
|
|
||
| return wrapper |
There was a problem hiding this comment.
Is anything stopping us from memoizing the output of _read_content_manifests() so we can move all these globals out of here? Django doesn't have a built in facility for caching generic Python objects … but there is django-cache-memoize, and a little pet memoize decoration in kolibri.core.notifications.utils.
There was a problem hiding this comment.
Memoizing could be part of a follow-up for persisting the collection manifest. Anyways I would keep that in a separate model. My intention was to decouple the collection manifest from the collection state. I wanted (but ran out of time) to come up with an EndlessKeyCollection class that didn't inherit from ContentManifest anymore. Instead:
class EndlessKeyCollection():
def __init__(self):
self.state = None # CollectionState object (Django model)
self.manifest = None # ContentManifest object| return Promise.all([currentCollectionExists(), getDownloadStatus(), getShouldResume()]).then( | ||
| ([currentExists, status, { shouldResume, name, sequence }]) => { | ||
| if (currentExists || _isDownloadComplete(status)) { |
| self.state, created = CollectionState.objects.get_or_create( | ||
| name=name, sequence=int(sequence) | ||
| ) |
There was a problem hiding this comment.
It looks like (if we move this below super().read(manifest_filename, validate)) we could do…
| self.state, created = CollectionState.objects.get_or_create( | |
| name=name, sequence=int(sequence) | |
| ) | |
| data = self._manifest_data.get("metadata", {}) | |
| self.state, _created = CollectionState.objects.get_or_create( | |
| name=name, sequence=int(sequence), defaults={ | |
| "title": data.get("title"), | |
| "subtitle": data.get("subtitle"), | |
| "description": data.get("description"), | |
| "required_gigabytes": int(data.get("required_gigabytes", 0)), | |
| } | |
| ) |
And then we could drop that whole if created block further down.
There was a problem hiding this comment.
Thanks for reviewing! I didn't know about the defaults in Model.get_or_create.
It stores: - The state of the installation. If content was imported, or only the metadata. If custom tags were applied. - The custom metadata from collection manifests (collection title, subtitle, description, etc). - If the collection is current. Ie. if it was the content pack initially downloaded. - If the collection is extra. It doesn't store the channel or the nodes information per se. That's the purpose of the manifest. The sequence is not treated as string anymore, except for the JSON manifest filename. Also: add the migration script created with `kolibri manage makemigrations` https://phabricator.endlessm.com/T35159
The initialization will do database handling after reading the manifest files. So add a decorator to all API views for initiating the global objects on first API call.
Adjust to the rename in the backend.
The collection state will set current after the download is completed.
Improve the iterator for tags and thumbnail jobs. In the same way as with the other iterators (metadata and content) these can be used to check if the installation phase was done (when they return an empty result). Add helper functions too. These helper functions can be used to set the installation state in the CollectionState model.
Change all occurrences of grade/name to name/sequence. The grade/name dates back to collections like "primary small". Now collections are identifyed by name/sequence like "artist 0001" Use the CollectionState model. Read it from the manifest files the first time. And update it in each installation stage. Once the installation is completed the is_current flag will be set to True. Also use "collection" as variable name instead of "manifest" when appropiate. Also: The custom metadata is now split in title/subtitle/description etc. https://phabricator.endlessm.com/T35159
manuq
force-pushed
the
T35159-persist-pack-selection
branch
from
5851a11 to
f170fec
Compare
https://phabricator.endlessm.com/T35159