Move some Rucio utils over to RucioUtils module #11218
Conversation
|
@mapellidario @belforte I don't remember if CRAB* services rely on the WMCore/Services/Rucio module, but in case it does, would this change be fine with you? |
|
thank Alana. Let me check latest code. |
|
As I thought, we decided to change and use directly Rucio client. Feel free to change. Yeah... I know that initially Eric asked us to use same as WMCore, but in the end we use slightly different things and few enouugh things anyhow and Rucio API is good enough that wrapping it does not gain us anything. No bad feeling. |
|
Jenkins results:
|
|
Thanks for prompt confirmation, Stefano. |
|
It just occurred to me that T0 might be using this as well. @germanfgv @jhonatanamado could you please check if you use it in any T0 specific code? |
|
Jenkins results:
|
|
test this please |
|
Jenkins results:
|
| # Python 3.6 includes something like this in the random library itself | ||
|
|
||
| total = sum(w for c, w in choices) | ||
| r = random.uniform(0, total) |
There was a problem hiding this comment.
I think single character variables are discouraged in Python and pylint will complain about it. I don't know how WMCore treat them, but I would like to mention it here during review and this function may need adjustment to fix that.
There was a problem hiding this comment.
I am actually pushing in a variation of it, dropping the python2 implementation in favor of the python3 random library.
| """ | ||
| # NOTE: a more reliable - but more expensive - way to know that would be | ||
| # to query `get_rse` and evaluate the rse_type parameter | ||
| return rseName.endswith("_Tape") |
There was a problem hiding this comment.
because you use this pattern at least twice and due to above comment I suggest to put rseName.endswith() into separate function which will check the rse name. Its implementation can be as simple as this return statement or more sophisticated as in a NOTE message.
|
Jenkins results:
|
|
Thank you for the prompt review, @vkuznet . You might want to look at the 3rd commit, which has a few changes on the code that you made a comment. |
|
Jenkins results:
|
|
test this please |
|
Jenkins results:
|
fix source docstring Replace custom weight choice by the random library
added unit test for weightedChoice function adapt unit test to fail less Update testWeightedChoice unit test test pylint
|
Jenkins results:
|
Fixes #11217
Status
ready
Description
This PR does not add any new code and/or logic, it simply moves some constants and functions from Services/Rucio to Services/RucioUtils new module.
Is it backward compatible (if not, which system it affects?)
NO (external clients will need to update their import, if needed)
Related PRs
None
External dependencies / deployment changes
None