fix(download): use record IDs for all local data paths#167
Conversation
) When downloading files using `--doi` or `--title` parameters, the download directory was incorrectly named "None" instead of the actual record ID. This commit fixes the problem by dereferencing DOI/title into record ID and using that for storing downloaded files. Closes cernopendata#166
tiborsimko
force-pushed
the
166-download-via-recid-and-xrootd
branch
from
5b29910 to
1414b7a
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #167 +/- ##
==========================================
+ Coverage 80.75% 80.88% +0.13%
==========================================
Files 12 12
Lines 717 722 +5
==========================================
+ Hits 579 584 +5
Misses 138 138
🚀 New features to boost your workflow:
|
cernopendata_client/cli.py
Outdated
|
|
||
| total_files = len(download_file_locations) | ||
| path = str(recid) | ||
| path = str(record_recid) |
There was a problem hiding this comment.
Is it still needed to convert to str? I assumed that record_json["metadata"]["recid"] is already a string
There was a problem hiding this comment.
Indeed this should not be needed anymore. Amended.
cernopendata_client/cli.py
Outdated
| @cernopendata_client.command() | ||
| @click.option("--recid", type=click.INT, help="Record ID") | ||
| @click.option("--doi", help="Digital Object Identifier") | ||
| @click.option("--title", help="Record title") |
There was a problem hiding this comment.
The DOI and title, are there exact searches or wildcard? It might help to describe that in the option
There was a problem hiding this comment.
Added to all CLI options.
| validate_recid(recid) | ||
|
|
||
| # Get record metadata and resolve recid from DOI/title if needed | ||
| record_json = get_record_as_json(server, recid, doi, title) |
There was a problem hiding this comment.
what about checking that record_json is defined? Or does the function throw an exception if there are no matches?
There was a problem hiding this comment.
Yes, the lookup does not allow to continue if there are no matches. It prints an error and sys-exits.
$ cernopendata-client download-files --recid 5500000
==> ERROR: The record ID number you supplied is not valid.| if os.path.isfile(test_file): | ||
| os.remove(test_file) | ||
|
|
||
|
|
There was a problem hiding this comment.
What about a test with an incorrect DOI?
There was a problem hiding this comment.
We already have a test for wrong DOI test_dry_run_from_doi_wrong() which does a dry run as well, but I guess that's OK and it should do.
) When downloading files using `--doi` or `--title` parameters, the download directory was incorrectly named "None" instead of the actual record ID. This commit fixes the problem by dereferencing DOI/title into record ID and using that for storing downloaded files. Closes cernopendata#166
tiborsimko
force-pushed
the
166-download-via-recid-and-xrootd
branch
from
1414b7a to
013bf0d
Compare
) When downloading files using `--doi` or `--title` parameters, the download directory was incorrectly named "None" instead of the actual record ID. This commit fixes the problem by dereferencing DOI/title into record ID and using that for storing downloaded files. Also enhances `--help` output to clarify that `--recid`, `--doi` and `--title` options are triggering exact match with no wildcards. Closes cernopendata#166
tiborsimko
force-pushed
the
166-download-via-recid-and-xrootd
branch
from
013bf0d to
9b97f7c
Compare
2 participants
When downloading files using
--doior--titleparameters, the download directory was incorrectly named "None" instead of the actual record ID. This commit fixes the problem by dereferencing DOI/title into record ID and using that for storing downloaded files.Closes #166