-
Notifications
You must be signed in to change notification settings - Fork 745
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
Content dir argument #9463
Conversation
@@ -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 |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
1 test needs an update due to the change in function signature breaking an assertion call. Linting is failing - I'd suggest installing |
4352ab9
to
c960961
Compare
c960961
to
4716380
Compare
I've just fixed the linting issues, sorry for that, I didn't have the |
81f87a6
to
8f351c4
Compare
Summary
Add
--content_dir
option to theimportchannel
andimportcontent
commands.This will allow to update content in an external device when running the application with the
CONTENT_FALLBACK_DIRS
configuration.