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
cli: Improve http/https dichotomy #93
Conversation
@@ -112,25 +112,6 @@ the **get-file-locations** command: | |||
|
|||
This command will output URIs for all the files associated with the record ID 5500, using the HTTP protocol. | |||
|
|||
**HTTPS protocol** |
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.
It is good to remove this section, but then we may not be having any HTTPS note anywhere. So you can add here:
Note that you can specify `--server https://opendata.cern.ch` if you would like to use the HTTPS protocol instead.
in those places where we spoke about HTTPS... In this way when people search the docs for HTTPS they would find at least a note on this.
tests/test_cli_download_files.py
Outdated
@@ -94,7 +94,7 @@ def test_download_files_https_pycurl(): | |||
os.remove(test_file) | |||
test_download_files = CliRunner() | |||
test_result = test_download_files.invoke( | |||
download_files, ["--recid", 3005, "--protocol", "https"] | |||
download_files, ["--recid", 3005, "--server", "https://opendata-dev.cern.ch"] |
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.
This would be the only use of opendata-dev.cern.ch
, so I think it is better to replace this with SERVER_HTTPS_URI
from config
so that we are using the same server consistently everywhere.
tests/test_cli_download_files.py
Outdated
@@ -112,7 +112,7 @@ def test_download_files_https_requests(mocker): | |||
os.remove(test_file) | |||
test_download_files = CliRunner() | |||
test_result = test_download_files.invoke( | |||
download_files, ["--recid", 3005, "--protocol", "https"] | |||
download_files, ["--recid", 3005, "--server", "https://opendata-dev.cern.ch"] |
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.
Ditto here.
Removed https protocol from protocol options, keeping only http or xrootd Accepting https with --server option closes #92
closes #92