Set default permission mode via stat values #19
Conversation
library/minecraft_server_file.py
Outdated
| @@ -59,7 +59,9 @@ def __init__(self, module): | |||
| self.path = os.path.expanduser(module.params['path']) | |||
| self.owner = module.params['owner'] | |||
| self.group = module.params['group'] | |||
| self.mode = module.params['mode'] if module.params['mode'] else 0644 | |||
| # Python version agnostic way of setting permissions 0644 | |||
| perm_0644 = stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | stat.S_IROTH | |||
There was a problem hiding this comment.
We require Python 2.7 so you can just write 0o644. Likely more understand than these flags.
There was a problem hiding this comment.
Oh, I didn't realize that format was compatible with both major versions. I agree it will be much more readable. I'll update the PR this evening.
|
Thanks for submitting the PR. I'll look at the Docker build. |
This time using python version agnostic octet formatting
|
Thanks! Was this the only Python 3 incompatibility you found? |
|
Yup! I ran pylint for python versions 2 and 3 against it to double check. It might be worth including a lint check in the CI tests. I can dedicate some time to that if you think it'd be worth while. |
|
Sounds great. tox makes that really easy. The Travis configuration is "generic", so we can't use tox-travis. But we can simply install it with Something like this would work: [tox]
envlist = py27,py35,py36
[testenv]
deps =
flake8
command = flake8 library/We could also run |
The existing value is invalid python3, and will result in a difficult to track runtime error. To support compatibility with python2.x and 3.x, I've changed this to use permission bitmasks from the stat library.
I figured I'd get approval from someone in the team before merging.