-
Notifications
You must be signed in to change notification settings - Fork 96
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Drop python 2 and 3.5 #174
Conversation
C901 | ||
C901, | ||
W503, | ||
W504 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
New entries are added to mute newer versions of flake8.
According to pycodestyle
E741: do not use variables named ‘l’, ‘O’, or ‘I’
E722: do not use bare except, specify exception instead
W503: line break before binary operator
W504: line break after binary operator
six==1.14.0 | ||
tabulate==0.8.6 | ||
vcrpy==4.0.2 | ||
pytest==5.3.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated using pip freeze --requirement requirements.txt
@@ -39,12 +39,11 @@ | |||
'Intended Audience :: Developers', | |||
'Intended Audience :: System Administrators', | |||
'Programming Language :: Python', | |||
'Programming Language :: Python :: 2', | |||
'Programming Language :: Python :: 2.7', | |||
'Programming Language :: Python :: 3', | |||
'Programming Language :: Python :: 3.5', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
3.5
tag is still retained, but not tested in CI.
self.assertFalse(bool(file_mode & stat.S_IXGRP)) | ||
self.assertFalse(bool(file_mode & stat.S_IROTH)) | ||
self.assertFalse(bool(file_mode & stat.S_IWOTH)) | ||
self.assertFalse(bool(file_mode & stat.S_IXOTH)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Windows doesn't have Linux-like user/group/other concepts for stat
. rwx
is always the same for all scopes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>>> import sys
>>> sys.version
'3.8.0 (tags/v3.8.0:fa919fd, Oct 14 2019, 19:37:50) [MSC v.1916 64 bit (AMD64)]'
As far as I understand from the doc, the limitation only takes effect on os.chmod
, however, os.stat
works for me. The doc didn't mention that os.stat
attributes won't be populated on Windows.
I agreed on file mode checking across different OS platform, but if file mode checking differentiate on Windows, should we add additional support?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also didn't find the doc for os.stat
's behavior on Windows.
st_mode=33206
is the default for a Windows file, which is 1000000 110 110 110
in binary. You can see rwx
is the same 110
for different scopes. However you change it, it keeps this pattern. For example, if you change the file to read-only via os.chmod('{}', stat.S_IRUSR)
, os.stat
is 33060
which is 1000000 100 100 100
in binary.
Checking permission on Windows requires win32security
according to https://stackoverflow.com/a/12168268/2199657. This will over complicate the test. Since we only save the config file under ~/.azure
, we can assume Windows behaves correctly unless the system is compromised, but of course this is out of the scope of the app itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe you are running this command on Cygwin which has additional file management logic. Running os.chmod
and os.stat
on bare Windows will repro my observation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, on CMD
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, installed and verified in local environment.
Drop python 2 and 3.5 as Azure CLI has dropped them: Azure/azure-cli#11363