Upload/download probe for rucio client #272 #273
Conversation
There was a problem hiding this comment.
So the probe does the right thing, that's good, thanks! Just a few minor comments regarding code style, nothing major. Also, can you switch to 4-spaces, instead of what I see as 8-tab?
PS: please rename the pull-request so we know what it is about ;-) also, this is a general rucio-clients test probe, not specific to ruciomover
tools/probes/common/check_ruciomover
Outdated
| Probe to check read/write with all protocols on target RSE with rucio client | ||
| ''' | ||
|
|
||
| import os |
There was a problem hiding this comment.
we sort our imports alphabetically
python native imports first
then python native imports with from clause
blank line
python external imports
python external from imports
blank line
rucio imports
rucio imports with from clause
tools/probes/common/check_ruciomover
Outdated
| :param protocol - transfer protocol | ||
| :param scope - file scope | ||
| :param lfn - file lfn | ||
| :param no_register - skip file registration in Rucio; also disables auto-deletion at lifetime expiration |
There was a problem hiding this comment.
missing one space on vertical alignment ;-)
tools/probes/common/check_ruciomover
Outdated
| :param scope: file scope | ||
| :param lfn: file lfn | ||
| :param dst: local destination directory | ||
| :param pfn: file pfn (required when the downloaded file is not registered in Rucio |
tools/probes/common/check_ruciomover
Outdated
| :param dst: local destination directory | ||
| :param pfn: file pfn (required when the downloaded file is not registered in Rucio | ||
| """ | ||
| pfnOption='' |
There was a problem hiding this comment.
variable names always_in_camel_case
tools/probes/common/check_ruciomover
Outdated
| raise Exception('stageIn failed -- rucio download did not succeed: %s' % o.replace('\n', '')) | ||
|
|
||
|
|
||
| OK, WARNING, CRITICAL, UNKNOWN = 0, 1, 2, 3 |
There was a problem hiding this comment.
should be moved into a python main
tools/probes/common/check_ruciomover
Outdated
| skip_download=False | ||
| if not args.download_uploaded: | ||
| repcli = ReplicaClient() | ||
| reps = [r for r in repcli.list_replicas([{'scope': args.download.split(':')[0], 'name': args.download.split(':')[1]}])] |
There was a problem hiding this comment.
split long lines on commas
tools/probes/common/check_ruciomover
Outdated
| if not args.download_uploaded: | ||
| repcli = ReplicaClient() | ||
| reps = [r for r in repcli.list_replicas([{'scope': args.download.split(':')[0], 'name': args.download.split(':')[1]}])] | ||
| if len(reps)==0 or rse not in reps[0]['rses'].keys(): |
There was a problem hiding this comment.
if not reps instead of len(reps)==0
tools/probes/common/check_ruciomover
Outdated
|
|
||
| report[prot['scheme']]={'upload':'UNKNOWN', 'download':'UNKNOWN'} | ||
|
|
||
| filename='%s.nagios-check_ruciomover.%s.%s.%s.%s' % (scope, rse, prot['scheme'], timestamp, fileuuid) |
tools/probes/common/check_ruciomover
Outdated
| os.unlink(filename) | ||
| continue | ||
|
|
||
| dst='download/%s.%s.%s' % (rse, prot['scheme'], timestamp) |
tools/probes/common/check_ruciomover
Outdated
|
|
||
| dst='download/%s.%s.%s' % (rse, prot['scheme'], timestamp) | ||
|
|
||
| pfntodownload=None |
nikmagini
changed the title
|
Just in case you haven't seen it, you can look at the Travis errors in detail. This is what's left: tools/probes/common/check_ruciomover:31:1: E302 expected 2 blank lines, found 1 tools/probes/common/check_ruciomover:54:1: E302 expected 2 blank lines, found 1 tools/probes/common/check_ruciomover:84:1: E302 expected 2 blank lines, found 1 tools/probes/common/check_ruciomover:85:9: F841 local variable 'WARNING' is assigned to but never used tools/probes/common/check_ruciomover:169:47: E231 missing whitespace after ':' tools/probes/common/check_ruciomover:169:69: E231 missing whitespace after ':' tools/probes/common/check_ruciomover:179:66: E231 missing whitespace after ':' tools/probes/common/check_ruciomover:179:80: E231 missing whitespace after ':' tools/probes/common/check_ruciomover:219:13: E303 too many blank lines (2) tools/probes/common/check_ruciomover:234:13: E303 too many blank lines (2) tools/probes/common/check_ruciomover:261:1: E305 expected 2 blank lines after class or function definition, found 1 |
|
Down to one problem :-) tools/probes/common/check_ruciomover:298:1: E305 expected 2 blank lines after class or function definition, found 1 |
|
you can install locally the pre-commit hook to get such errors before |
|
Thanks for the notification, fixed, somehow flake8 didn't report it for me... Anyway I'm now also setting up the development env properly |
|
can you do git fetch --all and then try again with pylint -E on your local install? |
nikmagini
force-pushed
the
patch-272-Probes__272__Add_ruciomover_probe
branch
from
85a05a7
to
c1e1618
Compare
|
Done, and also took the opportunity to squash my commits into a single one. I didn't get any errors from pylint -E, let's see if Travis agrees. |
Merge: 94fc77c 6b75e85