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

Content dir argument #9463

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
80 changes: 65 additions & 15 deletions kolibri/core/content/management/commands/importchannel.py
Expand Up @@ -21,10 +21,10 @@
COPY_METHOD = "copy"


def import_channel_by_id(channel_id, cancel_check):
def import_channel_by_id(channel_id, cancel_check, contentfolder=None):
try:
return channel_import.import_channel_from_local_db(
channel_id, cancel_check=cancel_check
channel_id, cancel_check=cancel_check, contentfolder=contentfolder
)
except channel_import.InvalidSchemaVersionError:
raise CommandError(
Expand Down Expand Up @@ -71,6 +71,12 @@ def add_arguments(self, parser):
action="store_true",
help="Only download database to an upgrade file path.",
)
network_subparser.add_argument(
"--content_dir",
type=str,
default=paths.get_content_dir_path(),
help="Download the database to the given content dir.",
)

local_subparser = subparsers.add_parser(
name="disk", cmd=self, help="Copy the content from the given folder."
Expand All @@ -88,22 +94,52 @@ def add_arguments(self, parser):
action="store_true",
help="Only download database to an upgrade file path.",
)
local_subparser.add_argument(
"--content_dir",
type=str,
default=paths.get_content_dir_path(),
help="Download the database to the given content dir.",
)

def download_channel(self, channel_id, baseurl, no_upgrade):
def download_channel(self, channel_id, baseurl, no_upgrade, content_dir):
logger.info("Downloading data for channel id {}".format(channel_id))
self._transfer(DOWNLOAD_METHOD, channel_id, baseurl, no_upgrade=no_upgrade)
self._transfer(
DOWNLOAD_METHOD,
channel_id,
baseurl,
no_upgrade=no_upgrade,
content_dir=content_dir,
)

def copy_channel(self, channel_id, path, no_upgrade):
def copy_channel(self, channel_id, path, no_upgrade, content_dir):
logger.info("Copying in data for channel id {}".format(channel_id))
self._transfer(COPY_METHOD, channel_id, path=path, no_upgrade=no_upgrade)

def _transfer(self, method, channel_id, baseurl=None, path=None, no_upgrade=False):
self._transfer(
COPY_METHOD,
channel_id,
path=path,
no_upgrade=no_upgrade,
content_dir=content_dir,
)

new_channel_dest = paths.get_upgrade_content_database_file_path(channel_id)
def _transfer(
self,
method,
channel_id,
baseurl=None,
path=None,
no_upgrade=False,
content_dir=None,
):

new_channel_dest = paths.get_upgrade_content_database_file_path(
channel_id, contentfolder=content_dir
)
dest = (
new_channel_dest
if no_upgrade
else paths.get_content_database_file_path(channel_id)
else paths.get_content_database_file_path(
channel_id, contentfolder=content_dir
)
)

# if new channel version db has previously been downloaded, just copy it over
Expand Down Expand Up @@ -131,7 +167,11 @@ def _transfer(self, method, channel_id, baseurl=None, path=None, no_upgrade=Fals

try:
self._start_file_transfer(
filetransfer, channel_id, dest, no_upgrade=no_upgrade
filetransfer,
channel_id,
dest,
no_upgrade=no_upgrade,
contentfolder=content_dir,
)
except transfer.TransferCanceled:
pass
Expand All @@ -149,7 +189,9 @@ def _transfer(self, method, channel_id, baseurl=None, path=None, no_upgrade=Fals
if os.path.exists(new_channel_dest) and not no_upgrade:
os.remove(new_channel_dest)

def _start_file_transfer(self, filetransfer, channel_id, dest, no_upgrade=False):
def _start_file_transfer(
self, filetransfer, channel_id, dest, no_upgrade=False, contentfolder=None
):
progress_extra_data = {"channel_id": channel_id}

with filetransfer, self.start_progress(
Expand All @@ -168,7 +210,9 @@ def _start_file_transfer(self, filetransfer, channel_id, dest, no_upgrade=False)
.exclude(kind=content_kinds.TOPIC)
.values_list("id", flat=True)
)
import_ran = import_channel_by_id(channel_id, self.is_cancelled)
import_ran = import_channel_by_id(
channel_id, self.is_cancelled, contentfolder
)
if node_ids and import_ran:
# annotate default channel db based on previously annotated leaf nodes
update_content_metadata(channel_id, node_ids=node_ids)
Expand All @@ -182,11 +226,17 @@ def _start_file_transfer(self, filetransfer, channel_id, dest, no_upgrade=False)
def handle_async(self, *args, **options):
if options["command"] == "network":
self.download_channel(
options["channel_id"], options["baseurl"], options["no_upgrade"]
options["channel_id"],
options["baseurl"],
options["no_upgrade"],
options["content_dir"],
)
elif options["command"] == "disk":
self.copy_channel(
options["channel_id"], options["directory"], options["no_upgrade"]
options["channel_id"],
options["directory"],
options["no_upgrade"],
options["content_dir"],
)
else:
self._parser.print_help()
Expand Down
32 changes: 27 additions & 5 deletions kolibri/core/content/management/commands/importcontent.py
Expand Up @@ -159,13 +159,25 @@ def add_arguments(self, parser):
dest="timeout",
help="Specify network timeout in seconds (default: %(default)d)",
)
network_subparser.add_argument(
"--content_dir",
type=str,
default=paths.get_content_dir_path(),
help="Download the content to the given content dir.",
)

disk_subparser = subparsers.add_parser(
name="disk", cmd=self, help="Copy the content from the given folder."
)
disk_subparser.add_argument("channel_id", type=str)
disk_subparser.add_argument("directory", type=str)
disk_subparser.add_argument("--drive_id", type=str, dest="drive_id", default="")
disk_subparser.add_argument(
"--content_dir",
type=str,
default=paths.get_content_dir_path(),
help="Copy the content to the given content dir.",
)

def download_content(
self,
Expand All @@ -178,6 +190,7 @@ def download_content(
import_updates=False,
fail_on_error=False,
timeout=transfer.Transfer.DEFAULT_TIMEOUT,
content_dir=None,
):
self._transfer(
DOWNLOAD_METHOD,
Expand All @@ -190,6 +203,7 @@ def download_content(
import_updates=import_updates,
fail_on_error=fail_on_error,
timeout=timeout,
content_dir=content_dir,
)

def copy_content(
Expand All @@ -202,6 +216,7 @@ def copy_content(
renderable_only=True,
import_updates=False,
fail_on_error=False,
content_dir=None,
):
self._transfer(
COPY_METHOD,
Expand All @@ -213,6 +228,7 @@ def copy_content(
renderable_only=renderable_only,
import_updates=import_updates,
fail_on_error=fail_on_error,
content_dir=content_dir,
)

def _transfer( # noqa: max-complexity=16
Expand All @@ -229,6 +245,7 @@ def _transfer( # noqa: max-complexity=16
import_updates=False,
fail_on_error=False,
timeout=transfer.Transfer.DEFAULT_TIMEOUT,
content_dir=None,
):
try:
if not import_updates:
Expand Down Expand Up @@ -274,8 +291,11 @@ def _transfer( # noqa: max-complexity=16
)
raise

if not content_dir:
content_dir = conf.OPTIONS["Paths"]["CONTENT_DIR"]

if not paths.using_remote_storage():
free_space = get_free_space(conf.OPTIONS["Paths"]["CONTENT_DIR"])
free_space = get_free_space(content_dir)

if free_space <= total_bytes_to_transfer:
raise InsufficientStorageSpaceError(
Expand Down Expand Up @@ -377,7 +397,9 @@ def _transfer( # noqa: max-complexity=16
f = files_to_download.pop()
filename = get_content_file_name(f)
try:
dest = paths.get_content_storage_file_path(filename)
dest = paths.get_content_storage_file_path(
filename, contentfolder=content_dir
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: maybe it's better to use the same variable name everywhere, eg renaming the new CLI parameter to --content-folder. Or do you prefer to use "content dir" as the exposed name and "contentfolder" as the internal name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CONTENT_DIR is the configuration option and it's used in other places, so I think it's better to use content-dir for this option.

)
except InvalidStorageFilenameError:
# If the destination file name is malformed, just stop now.
overall_progress_update(f["file_size"])
Expand Down Expand Up @@ -438,9 +460,7 @@ def _transfer( # noqa: max-complexity=16
file_checksums_to_annotate.append(f["id"])
transferred_file_size += f["file_size"]
remaining_bytes_to_transfer -= f["file_size"]
remaining_free_space = get_free_space(
conf.OPTIONS["Paths"]["CONTENT_DIR"]
)
remaining_free_space = get_free_space(content_dir)
if remaining_free_space <= remaining_bytes_to_transfer:
raise InsufficientStorageSpaceError(
"Kolibri ran out of storage space while importing content"
Expand Down Expand Up @@ -566,6 +586,7 @@ def handle_async(self, *args, **options):
import_updates=options["import_updates"],
fail_on_error=options["fail_on_error"],
timeout=options["timeout"],
content_dir=options["content_dir"],
)
elif options["command"] == "disk":
self.copy_content(
Expand All @@ -577,6 +598,7 @@ def handle_async(self, *args, **options):
renderable_only=options["renderable_only"],
import_updates=options["import_updates"],
fail_on_error=options["fail_on_error"],
content_dir=options["content_dir"],
)
else:
self._parser.print_help()
Expand Down
5 changes: 4 additions & 1 deletion kolibri/core/content/test/test_import_export.py
Expand Up @@ -21,6 +21,7 @@
from kolibri.core.content.models import ContentNode
from kolibri.core.content.models import File
from kolibri.core.content.models import LocalFile
from kolibri.core.content.utils import paths
from kolibri.core.content.utils.content_types_tools import (
renderable_contentnodes_q_filter,
)
Expand Down Expand Up @@ -209,7 +210,9 @@ def test_remote_import_full_import(
call_command("importchannel", "network", "197934f144305350b5820c7c4dd8e194")
is_cancelled_mock.assert_called()
import_channel_mock.assert_called_with(
"197934f144305350b5820c7c4dd8e194", cancel_check=is_cancelled_mock
"197934f144305350b5820c7c4dd8e194",
cancel_check=is_cancelled_mock,
contentfolder=paths.get_content_dir_path(),
)

@patch(
Expand Down
17 changes: 12 additions & 5 deletions kolibri/core/content/utils/channel_import.py
Expand Up @@ -215,13 +215,16 @@ def __init__(
cancel_check=None,
source=None,
destination=None,
contentfolder=None,
):
self.channel_id = channel_id
self.channel_version = channel_version

self.cancel_check = cancel_check

self.source_db_path = source or get_content_database_file_path(self.channel_id)
self.source_db_path = source or get_content_database_file_path(
self.channel_id, contentfolder=contentfolder
)

self.source = Bridge(sqlite_file_path=self.source_db_path)

Expand Down Expand Up @@ -1079,10 +1082,11 @@ class InvalidSchemaVersionError(Exception):


def initialize_import_manager(
channel_id, cancel_check=None, source=None, destination=None
channel_id, cancel_check=None, source=None, destination=None, contentfolder=None
):
channel_metadata = read_channel_metadata_from_db_file(
source or get_content_database_file_path(channel_id)
source
or get_content_database_file_path(channel_id, contentfolder=contentfolder)
)
# For old versions of content databases, we can only infer the schema version
min_version = channel_metadata.get(
Expand Down Expand Up @@ -1121,11 +1125,14 @@ def initialize_import_manager(
cancel_check=cancel_check,
source=source,
destination=destination,
contentfolder=contentfolder,
)


def import_channel_from_local_db(channel_id, cancel_check=None):
import_manager = initialize_import_manager(channel_id, cancel_check=cancel_check)
def import_channel_from_local_db(channel_id, cancel_check=None, contentfolder=None):
import_manager = initialize_import_manager(
channel_id, cancel_check=cancel_check, contentfolder=contentfolder
)

import_ran = import_manager.import_channel_data()

Expand Down