default stageout commands for protocols #12367
Conversation
|
Jenkins results:
|
|
Jenkins results:
|
|
Jenkins results:
|
amaltaro
left a comment
amaltaro
left a comment
There was a problem hiding this comment.
@nhduongvn thank you for providing these changes.
In addition to the comments left along the code, would it be possible to create unit test(s) to validate this new method? I see we have a few storage.json and site-local-config.xml under: https://github.com/dmwm/WMCore/tree/master/test/python/WMCore_t/Storage_t
It would be great to have one test validating the rules option and one validating the prefix one. Perhaps one that won't match anything as well, defaulting to gfal2.
| :path: a LFN path, for example /store/abc/xyz.root | ||
| :style: type of conversion. lfn-to-pfn is to convert LFN to PFN and pfn-to-pfn is for PFN to LFN | ||
| :caller is the method from there this method was called. It's used for resolving chained rules. When a rule is chained, the path translation of protocol defined in "chain" attribute should be applied first before the one specified in this rule. Here is an example. In this storage description, https://gitlab.cern.ch/SITECONF/T1_DE_KIT/-/blob/master/storage.json, the rule of protocol WebDAV of volume KIT_MSS is chained to the protocol pnfs of the same volume. The path translation of WebDAV rule must be done by applying the path translation of pnfs rule first before its own path translation is applied. | ||
| :caller: is the method from there this method was called. It's used for resolving chained rules. |
There was a problem hiding this comment.
Did you mean "where" instead of "there"?
| with open(storageJsonName, encoding="utf-8") as jsonFile: | ||
| jsElements = json.load(jsonFile) | ||
| except Exception as ex: | ||
| msg = "RucioFileCatalog.py:getDefaultCmd() Error reading storage.json: %s\n" % storageJsonName |
There was a problem hiding this comment.
Can you please update this error message to keep consistent with the actual method name?
Or maybe I would suggest to rephrase it to something like (the module and line number are already present in the logs):
msg = f"Failed to open storage.json: {storageJsonName}\n. Error: {str(ex)}"
| url_scheme = urlparse(proto["prefix"]).scheme | ||
|
|
||
| # Map scheme to command | ||
| return { |
There was a problem hiding this comment.
I would suggest moving this map to the top of this module, as a global variable, such that it is easier to identify this map in the code. Perhaps:
STAGEOUT_PROTOCOL_MAP = 'root': 'xrdcp',
'davs': 'gfal2',
'file': 'cp'
}
| 'file': 'cp' | ||
| }.get(url_scheme, 'gfal2') | ||
|
|
||
| break # matching site+volume found and processed |
There was a problem hiding this comment.
I would suggest adding a log record here to say that the json was processed but no matches were found.
Note that log syntax in this module seems to be different, so we could probably use something like:
logging.log(logging.WARNING, message)
|
|
||
| break # matching site+volume found and processed | ||
|
|
||
| return None #no matched protocol so command is None |
There was a problem hiding this comment.
Same for this line, I would add a logging.ERROR report lack of information (site/volume etc).
There was a problem hiding this comment.
@amaltaro, I addressed all your comments
|
Jenkins results:
|
3 participants
Fixes #12314
Status
Ready
Description
Add a function, get_default_cmd, in RucioFileCatalog.py to identify the default command from the stageout command
Is it backward compatible (if not, which system it affects?)
NO. The "command" definition in stageout methods will be ineffective