Conversation
src/check_file_age.py
Outdated
| args = get_parser().parse_args() | ||
|
|
||
| if not args.path: | ||
| print('UNKNOWN - no path supplied') |
There was a problem hiding this comment.
Why not making the argument required then?
src/check_file_age.py
Outdated
| print('UNKNOWN - no path supplied') | ||
| exit(3) | ||
|
|
||
| age = 0 |
There was a problem hiding this comment.
No need to neither explicitly initialise nor declare it. age will be available outside the try scope too.
src/check_file_age.py
Outdated
| try: | ||
| age = time.time() - os.path.getmtime(args.path) | ||
| except OSError: | ||
| print('UNKNOWN - {} is not present or no permissions'.format( |
There was a problem hiding this comment.
Apply Multiline style code:
func(
argument_1, argument_2,
'some small string format'.format(...),
'some bigger bigger bigger string format'
.format(...)
even_more_arguments, ...
)
src/check_file_age.py
Outdated
| status = 'OK' | ||
| code = 0 | ||
|
|
||
| if age > args.critical*60: |
There was a problem hiding this comment.
PEP8: Spaces around operators.
src/check_file_age.py
Outdated
| if age > args.critical*60: | ||
| code = 2 | ||
| status = 'CRITICAL' | ||
| elif age > args.warning*60: |
There was a problem hiding this comment.
PEP8: Spaces around operators.
src/check_file_age.py
Outdated
| '--warning', '-w', | ||
| help='warning threshold in minutes for file age', | ||
| default=30, type=int) | ||
| parser.add_argument( |
There was a problem hiding this comment.
Apply Multiline style code:
func(
argument_1, argument_2,
'some small string format'.format(...),
'some bigger bigger bigger string format'
.format(...)
even_more_arguments, ...
)
src/check_file_age.py
Outdated
| parser.add_argument('--path', '-p', help='path of file to check') | ||
| parser.add_argument( | ||
| '--warning', '-w', | ||
| help='warning threshold in minutes for file age', |
There was a problem hiding this comment.
Make help being the last argument standing alone.
src/check_file_age.py
Outdated
| default=30, type=int) | ||
| parser.add_argument( | ||
| '--critical', '-c', | ||
| help='critical threshold in minutes for file age', |
There was a problem hiding this comment.
Make help being the last argument standing alone.
TheFRedFox
left a comment
TheFRedFox
left a comment
There was a problem hiding this comment.
Just minor things except of the sys.exit one. Already approving though. Changes would be appreciated. ;)
| '--path', '-p', | ||
| required=True, |
There was a problem hiding this comment.
Can be just:
| '--path', '-p', | |
| required=True, | |
| '--path', '-p', required=True, |
| '--warning', '-w', | ||
| default=30, type=int, |
There was a problem hiding this comment.
| '--warning', '-w', | |
| default=30, type=int, | |
| '--warning', '-w', type=int, default=30, |
| '--critical', '-c', | ||
| default=60, type=int, |
There was a problem hiding this comment.
| '--critical', '-c', | |
| default=60, type=int, | |
| '--critical', '-c', type=int, default=60, |
| print( | ||
| 'UNKNOWN - {} is not present or no permissions'.format(args.path) | ||
| ) |
There was a problem hiding this comment.
We rather go with this multiline style:
| print( | |
| 'UNKNOWN - {} is not present or no permissions'.format(args.path) | |
| ) | |
| print( | |
| 'UNKNOWN - {} is not present or no permissions'.format(args.path) | |
| ) |
| '{} - {} last changed {} minutes ago'.format( | ||
| status, args.path, round(age / 60) | ||
| ) |
There was a problem hiding this comment.
Try if you like this way more:
| '{} - {} last changed {} minutes ago'.format( | |
| status, args.path, round(age / 60) | |
| ) | |
| '{} - {} last changed {} minutes ago' | |
| .format(status, args.path, round(age / 60)) |
| status, args.path, round(age / 60) | ||
| ) | ||
| ) | ||
| exit(code) |
There was a problem hiding this comment.
Please use sys.exit. quit and exit are rather used for interactive shells. You will also need to import sys for that.
4 participants
This check was written by Jon Dawson.