Skip content import #900
Merged
Skip content import #900
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is already ignored by pre-commit because it's not under version control, but if you run bare `flake8` it will check it.
These were helpful when I was developing that code, but now they just add a lot of noise to the test output.
In order for `ContentServer` to support the `/api/public/v1/channels/lookup/<channel_id>` endpoint, it needs to be able to introspect the channel database. Ideally this would open the actual sqlite database and use Kolibri's routines, but that would be complicated. Instead, copy the input JSON files into the content directory so the server can get the channel data from them without using the databases.
When the `ContentServer` is run in a separate process with `multiprocessing`, none of the log messages are recorded. Instead, use a separate thread in the current process. All that needs to happen: * `serve_forever()` is run in a separate thread * `shutdown()` is called from the main thread * `server_close()` is called to close the socket Now that the log messages are visible, it's clear that the thread name in the handler log message is redundant since our pytest default format includes the thread name.
Unfortunately, even if Kolibri already has all the content nodes, it will still probe the remote server for channel metadata. Since that fails when the device is offline, skip creating the tasks if it appears all the content nodes are available. This uses the same `get_import_export_data` helper that Kolibri uses when determining nodes to download. That's an expensive query, but it appears that's the only way to reliably determine if a download is needed or not. Fixes: #890
dylanmccall
reviewed
Nov 15, 2023
dylanmccall
approved these changes
Nov 15, 2023
In order to import an extra channel and its thumbnails, the storage hook would detect a completed channel import task and then create a thumbnail import task for that channel dynamically. However, now that channel import tasks are skipped if the channel is already available, no completed channel import task would arrive to trigger the thumbnail task creation. That's an unlikely scenario since the installation is expected to come with either no content or fully populated channels, but it can be handled better. Instead of using 2 tasks, use a single `remoteimport` task that combines `remotechannelimport` and `remotecontentimport`. The hook action is kept in case there are existing installations that hadn't completed the background tasks yet, but hopefully it can be removed someday.
Mocking these interfaces was hiding the fact that Kolibri was still making network requests when they weren't expected. Instead, have `ContentServer` handle them so tests that aren't supposed to make network requests fail.
dbnicholson
force-pushed
the
skip-content-import
branch
from
b1541de to
ffaa29c
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Like was done with channel imports, skip content import tasks if all the resources are already available. This is needed when the device is online because Kolibri will probe the channel data from the server regardless of whether content is needed or not. While working on this, I noticed that extra channel thumbnail downloads may be skipped if the channel was already present.
Sorry this is a bit big. I wanted to be sure there really wouldn't be any network requests if they weren't needed. That involved a bit of work with the test studio server so that it could respond to all the requests without having them stubbed out.
Fixes: #890