Skip to content

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.

Sign up for GitHub

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

Merged
jiasli merged 5 commits into from
Feb 20, 2020
Merged

Drop python 2 and 3.5 #174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b001ee1
drop python 2
jiasli Feb 17, 2020
ff09823
update package versions
jiasli Feb 17, 2020
e66c828
update
jiasli Feb 17, 2020
ba2c527
fix flake8
jiasli Feb 17, 2020
39ffa91
fix test on Windows
jiasli Feb 17, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion .flake8
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ max-complexity = 10
ignore =
E126,
E501,
E741,
E722,
F401,
F811,
C901
C901,
W503,
W504
Copy link
Member Author

@jiasli jiasli Feb 18, 2020 •
edited
Loading

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

4 changes: 2 additions & 2 deletions .travis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
sudo: false
language: python
python:
- '2.7'
- '3.5'
- '3.6'
- '3.7'
- '3.8'
install: pip install tox-travis
script: tox
deploy:
Expand Down
25 changes: 12 additions & 13 deletions requirements.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,12 @@
argcomplete==1.8.2
colorama==0.3.9
flake8==3.2.1
jmespath==0.9.2
mock==2.0.0
pylint==1.9.4; python_version <= '2.7'
pylint==2.3.1; python_version >= '3.5'
pygments==2.2.0
pyyaml==3.12
six==1.10.0
tabulate==0.7.7
vcrpy==1.10.3
pytest==4.6.6
argcomplete==1.11.1
colorama==0.4.3
flake8==3.7.9
jmespath==0.9.4
mock==4.0.1
pylint==2.3.0
Pygments==2.5.2
PyYAML==5.3
six==1.14.0
tabulate==0.8.6
vcrpy==4.0.2
pytest==5.3.5
Copy link
Member Author

@jiasli jiasli Feb 18, 2020

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

3 changes: 1 addition & 2 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Copy link
Member Author

@jiasli jiasli Feb 18, 2020 •
edited
Loading

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.

'Programming Language :: Python :: 3.6',
'Programming Language :: Python :: 3.7',
'Programming Language :: Python :: 3.8',
'License :: OSI Approved :: MIT License',
],
packages=['knack', 'knack.testsdk'],
Expand Down
16 changes: 9 additions & 7 deletions tests/test_config.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,13 +186,15 @@ def test_set_config_value_file_permissions(self):
file_mode = os.stat(self.cli_config.config_path).st_mode
self.assertTrue(bool(file_mode & stat.S_IRUSR))
self.assertTrue(bool(file_mode & stat.S_IWUSR))
self.assertFalse(bool(file_mode & stat.S_IXUSR))
self.assertFalse(bool(file_mode & stat.S_IRGRP))
self.assertFalse(bool(file_mode & stat.S_IWGRP))
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))
# only S_IRUSR and S_IWUSR are supported on Windows: https://docs.python.org/3.8/library/os.html#os.chmod
if os.name != 'nt':
self.assertFalse(bool(file_mode & stat.S_IXUSR))
self.assertFalse(bool(file_mode & stat.S_IRGRP))
self.assertFalse(bool(file_mode & stat.S_IWGRP))
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))
Copy link
Member Author

@jiasli jiasli Feb 18, 2020

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.

Copy link

@haroldrandom haroldrandom Feb 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

>>> 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?

Copy link
Member Author

@jiasli jiasli Feb 20, 2020

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.

Copy link
Member Author

@jiasli jiasli Feb 20, 2020

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.

Copy link

@haroldrandom haroldrandom Feb 20, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, on CMD


def test_has_option_local(self):
section = 'MySection'
Expand Down
2 changes: 1 addition & 1 deletion tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[tox]
envlist = py27,py35,py36,py37
envlist = py36,py37,py38
[testenv]
deps = -rrequirements.txt
commands=
Expand Down