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

Conversation

danigm
Copy link
Contributor

@danigm danigm commented May 27, 2022

Summary

Add --content_dir option to the importchannel and importcontent commands.

This will allow to update content in an external device when running the application with the CONTENT_FALLBACK_DIRS configuration.

@radinamatic radinamatic added the TODO: needs user docs Requires user docs update label May 27, 2022
@@ -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.

@rtibbles
Copy link
Member

rtibbles commented Jun 1, 2022

1 test needs an update due to the change in function signature breaking an assertion call.

Linting is failing - I'd suggest installing pre-commit to prevent linting errors in future.

@danigm
Copy link
Contributor Author

danigm commented Jun 6, 2022

1 test needs an update due to the change in function signature breaking an assertion call.

Linting is failing - I'd suggest installing pre-commit to prevent linting errors in future.

I've just fixed the linting issues, sorry for that, I didn't have the pre-commit enabled. I've enabled it now for future PRs.

@rtibbles rtibbles merged commit a50f6b0 into learningequality:release-v0.15.x Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TODO: needs user docs Requires user docs update
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants