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.

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

Use manifest.json files to download collections #436

Merged
merged 9 commits into from
Jul 14, 2022
Merged

Use manifest.json files to download collections #436

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ed1fdb1
Use manifest.json files to download collections
danigm Jun 29, 2022
523780c
Fallback collection download to remote by token
danigm Jul 8, 2022
63cdfa6
Use collections by grade
danigm Jul 11, 2022
5f80a8f
Fix InstallContentModal visibility at launch
danigm Jul 13, 2022
53cf7b2
Reload ContentNodeResource on reloadChannels
danigm Jul 13, 2022
195f340
Improve download job handling
danigm Jul 13, 2022
4068f9b
Show percentage in the Download progress bar
danigm Jul 13, 2022
5cadfc7
Use .values() instead of .items() in collection view
danigm Jul 14, 2022
c29aeae
Show error if collection manifest is not there
danigm Jul 14, 2022
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
Prev Previous commit
Next Next commit
Fallback collection download to remote by token 
This patch install the collection using the token if the manifest file
is not present or can't be found.

https://phabricator.endlessm.com/T33570
  • Loading branch information
@danigm
danigm committed Jul 14, 2022
commit 523780ca2c6814e430849e99c9377f307dba1c6a
26 changes: 18 additions & 8 deletions kolibri_explore_plugin/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from django.views.generic.base import TemplateView
from django.views.generic.base import View
from kolibri.core.content.api import cache_forever
from kolibri.core.content.api import RemoteChannelViewSet
from kolibri.core.content.zip_wsgi import add_security_headers
from kolibri.core.content.zip_wsgi import get_embedded_file
from kolibri.core.decorators import cache_no_user_data
Expand Down Expand Up @@ -209,16 +210,23 @@ def post(self, request):
COLLECTION_PATHS, f"{collection}.json"
)

if not os.path.exists(collection_manifest):
raise Http404
if os.path.exists(collection_manifest):
manifest = {}
with open(collection_manifest) as f:
manifest = json.load(f)
channels = manifest.get("channels", [])
else:
# Fallback, download full collection using the token
token = self.COLLECTIONS[collection]["token"]

manifest = {}
with open(collection_manifest) as f:
manifest = json.load(f)
channel_viewset = RemoteChannelViewSet()
channels = channel_viewset._make_channel_endpoint_request(
identifier=token
)

job_ids = []
pid, _, _ = get_status()
for channel in manifest.get("channels", []):
for channel in channels:
task = {
"channel_id": channel["id"],
"baseurl": self.BASE_URL,
Expand All @@ -231,8 +239,10 @@ def post(self, request):
_remoteimport,
task["channel_id"],
task["baseurl"],
node_ids=channel["include_node_ids"] or None,
exclude_node_ids=channel["exclude_node_ids"] or None,
# Done this way to convert [] to None
node_ids=channel.get("include_node_ids") or None,
# Done this way to convert [] to None
exclude_node_ids=channel.get("exclude_node_ids") or None,
Comment on lines +276 to +279
Copy link
Collaborator

@dylanmccall dylanmccall Jul 13, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

👍 there's a subtle difference in how node_ids=[] is handled between the code that copies data and the code that marks nodes as available, so this is a useful way to deal with that.

It's worth noting this behaviour will be different with learningequality/kolibri#9460. Setting include_node_ids: [] is the same as saying "no nodes", while include_node_ids: None means "the default, which is all of the nodes".

Once we have Kolibri with my changes applied, the manifest file code should actually never create a manifest file with the latter: if it includes all the nodes from a channel, it ends up with include_node_ids: [${channel_id}]. If a manifest file has include_node_ids: [], it's probably wrong but there's no need arguing with it.

Hopefully in a future iteration we can pass a manifest file directly to the remoteimport task so we don't need to deal with all this :)

extra_metadata=task,
track_progress=True,
cancellable=True,
Expand Down