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.

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

endlesskey: Import only specified content in collections #141

Merged
merged 3 commits into from
Dec 19, 2023

Conversation

dylanmccall
Copy link
Contributor

The previous behaviour, of importing all content from all channels in the collections, is preserved by setting include_full_channels to true.

https://phabricator.endlessm.com/T35103

The previous behaviour, of importing all content from all channels in
the collections, is preserved by setting include_full_channels to true.

https://phabricator.endlessm.com/T35103
@dylanmccall dylanmccall marked this pull request as ready for review December 15, 2023 18:17
Copy link
Member

@dbnicholson dbnicholson left a comment

Choose a reason for hiding this comment

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

This looks good modulo one comment. I'm on the fence about changing the default since it requires a synchronized change in endless-image-config, but it's not that big of a deal.

curious
explorer
inventor
scientist
Copy link
Member

Choose a reason for hiding this comment

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

I think you want to also include extras or this won't work offline. The explore plugin will try to import all the thumbnails from nodes in the extras pack at runtime. See https://github.com/endlessm/kolibri-explore-plugin/blob/master/kolibri_explore_plugin/collectionviews.py#L43. I think it's fine to import whatever nodes are specified in it.

Copy link
Contributor Author

@dylanmccall dylanmccall Dec 16, 2023

Choose a reason for hiding this comment

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

Oh yeah, that's a good one. I pushed a change just now (4583308) getting it to download all thumbnails for all the included channels.

Alas, adding the extras collection is a bit problematic, since that manifest file includes a bunch of content as well. But it probably shouldn't? I proposed endlessm/endless-key-collections#37 just now to address that, and then I can add it here.

Copy link
Member

Choose a reason for hiding this comment

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

The collections change was merged, released, and included in an org.endlessos.Key flathub release. So, I think this should be good now.

Copy link
Member

@dbnicholson dbnicholson left a comment

Choose a reason for hiding this comment

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

Sorry, meant that to be request changes.

@dylanmccall dylanmccall force-pushed the T35103-endless-key-collections branch from 7154636 to 75060ae Compare December 16, 2023 01:26
@dylanmccall dylanmccall force-pushed the T35103-endless-key-collections branch from 75060ae to 4583308 Compare December 19, 2023 01:16
curious
explorer
inventor
scientist
Copy link
Member

Choose a reason for hiding this comment

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

The collections change was merged, released, and included in an org.endlessos.Key flathub release. So, I think this should be good now.

kolibri manage --skip-update \
importcontent --include-unrenderable-content --fail-on-error \
--node_ids="" --all-thumbnails \
network "${channel}"
Copy link
Member

Choose a reason for hiding this comment

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

IMO, this should only be in the not full channels branch below. The full channels branch is obviously already going to import all the thumbnails. Not a big deal, though.

@dbnicholson dbnicholson merged commit fd0382c into master Dec 19, 2023
2 checks passed
@dbnicholson dbnicholson deleted the T35103-endless-key-collections branch December 19, 2023 17:21
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.

2 participants