OneSdsc usability: specifying locations (popeye/SDSC) by kwarg, rest caching disabled by default#121
OneSdsc usability: specifying locations (popeye/SDSC) by kwarg, rest caching disabled by default#121grg2rsr wants to merge 3 commits into
Conversation
… use from popeye or from datauser/SDSC
…tantiate an incompatible combination of location and cache_dir kwargs
| # Ensure parquet tables downloaded to separate location to the dataset repo | ||
| kwargs['tables_dir'] = one.params.get_cache_dir() # by default this is user downloads | ||
| kwargs["tables_dir"] = one.params.get_cache_dir() # by default this is user downloads | ||
| if disable_rest_caching: |
There was a problem hiding this comment.
Better to overload the original kwarg rather than adding a new one to avoid parameter conflicts. You could simply add cache_rest=None to the init signature.
| raise ValueError("both location and cache dir are specified, and they are incompatible") | ||
| cache_dir = CACHE_DIR_SDSC | ||
|
|
||
| def __init__(self, *args, cache_dir=CACHE_DIR, **kwargs): |
There was a problem hiding this comment.
Looks like you've changed the default to popeye. You should make others aware of this.
Also all previous code will raise a value error because neither cache_dir, nor location are None by default so if they pass in any other cache dir path without passing location=None they're get a value error. Personally I find having two related parameters confusing and a little redundant given that you've already renamed the cache dir variables to clearer names. If this location field is not used for anything else I'd rather remove it
2 participants
The
OneSdscclass does, as it is now, not work out of the box for datauser on SDSC because of differences in the default cache dir.This patch:
SDSCvspopeye, and handles the setting of the correct cache dircache_rest=Nonehas to be passed when usingOneSdscon popeye (as users on popeye do not have write permission on the cache directory). Datauser on SDSC does, and rest caching can be enabled by setting a kwarg explicitly if it is required.