-
Notifications
You must be signed in to change notification settings - Fork 98
Drop python 2 and 3.5 #174
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
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
| '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
fengzhou-msft
left a comment
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