Skip to content
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

[hold until 3.6 EOL] Bump min Python version from 3.6.2 to 3.7.2 #260

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

c0llab0rat0r
Copy link
Contributor

@c0llab0rat0r c0llab0rat0r commented Apr 5, 2021

  1. Drop support for 3.5 and 3.6
  2. Remove conditional shimming for 3.5 and 3.6
  3. Change some typing from comment style to 3.7 style
  4. Suppress the mypy warnings in returns_single_item and returns_multiple_items that were causing a pre-existing build failure
  5. Run some of the build steps on focal (defaults to Python 3.8) that were running on bionic (defaults to Python 3.6)

There's still quite a few typing warning suppression; this commit doesn't attempt to include a sweeping correction to those. Rather, it gets the library up to Python 3.7, which will make a sweeping correction easier.

@c0llab0rat0r
Copy link
Contributor Author

@ntninja I'm pretty close on this, but as you can see, I'm having some trouble with the last mile.

I ran into two problems:

  1. pypy3 build did not work anymore. I believe this to be caused by travis only having Python 3.6, and not having a newer version of pypy that supports Python 3.7 syntax. I resolved this by commenting pypy out of the build
  2. httpcore does not import when running the httpx build. I tried to see if I could get the http_requests flavor to work instead (there's a code change to the imports to sidestep httpx) and that resulted in coverage errors (89% < 90%)

I feel like I'm close but I need some help to push it across the finish line.

@c0llab0rat0r
Copy link
Contributor Author

Holy hell, the build finally "passes" - except for the style linting, which is fixed in #257, and should be merged first.

A code review would be much appreciated.

@c0llab0rat0r
Copy link
Contributor Author

I would expect this change to get squashed to remove some of my commit noise and silly comments.

Copy link
Contributor

@ntninja ntninja left a comment

Choose a reason for hiding this comment

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

Thank you, this looks pretty good overall! As mentioned, Python 3.6 support is still important through so that will have to be added back. Otherwise it's only minor stuff.

.travis.yml Show resolved Hide resolved
.travis.yml Show resolved Hide resolved
ipfshttpclient/__init__.py Outdated Show resolved Hide resolved
ipfshttpclient/client/base.py Outdated Show resolved Hide resolved
ipfshttpclient/client/base.py Outdated Show resolved Hide resolved
ipfshttpclient/filescanner.py Outdated Show resolved Hide resolved
ipfshttpclient/filescanner.py Outdated Show resolved Hide resolved
ipfshttpclient/http_common.py Outdated Show resolved Hide resolved
test/unit/test_encoding.py Outdated Show resolved Hide resolved
test/unit/test_swarm.py Outdated Show resolved Hide resolved
@c0llab0rat0r
Copy link
Contributor Author

Build is green after rebase and incorporating code review feedback.

It doesn't have the 3.7 -> 3.6 code review feedback yet, which I'll get to once I have a larger block of time available.

I'll incorporate smaller code review feedback items as they're posted.

@ntninja
Copy link
Contributor

ntninja commented Apr 6, 2021

Build is green after rebase and incorporating code review feedback.

Awesome! Thank you! Don't overwork yourself or anything (just let me know if you don't feel like finishing something)! 😉

@c0llab0rat0r c0llab0rat0r force-pushed the require-python-37 branch 4 times, most recently from 1fcafe8 to 21718d7 Compare April 9, 2021 18:42
@c0llab0rat0r c0llab0rat0r changed the title Bump min Python version from 3.5.4 to 3.7.2 Bump min Python version from 3.6.2 to 3.7.2 Apr 9, 2021
c0llab0rat0r added a commit to c0llab0rat0r/py-ipfs-http-client that referenced this pull request Apr 9, 2021
@c0llab0rat0r
Copy link
Contributor Author

Transient docker failures on the Windows builds.

c0llab0rat0r added a commit to c0llab0rat0r/py-ipfs-http-client that referenced this pull request Apr 9, 2021
@c0llab0rat0r
Copy link
Contributor Author

Another transient docker failure on a Windows build.

c0llab0rat0r added a commit to c0llab0rat0r/py-ipfs-http-client that referenced this pull request Apr 9, 2021
@c0llab0rat0r
Copy link
Contributor Author

Another transient Windows build failure, this time with a link:

https://travis-ci.com/github/ipfs-shipyard/py-ipfs-http-client/jobs/497459752

They all looked like this one:

 	# Wait for daemon process to disappear

	for _ in range(10000):
		if not daemon_is_running():
			break

		time.sleep(0.001)

	# Daemon should not be running anymore
>   	assert not daemon_is_running()
E    assert not True
E     +  where True = <function test_daemon_stop.<locals>.daemon_is_running at 0x0000020AFD70BDC8>()
test\functional\test_miscellaneous.py:47: AssertionError

============================== warnings summary ===============================

test\functional\test_miscellaneous.py:23

C:\Users\travis\build\ipfs-shipyard\py-ipfs-http-client\test\functional\test_miscellaneous.py:23: PytestUnknownMarkWarning: Unknown pytest.mark.last - is this a typo?  You can register custom marks to avoid this warning - for details, see https://docs.pytest.org/en/latest/mark.html

@pytest.mark.last

c0llab0rat0r added a commit to c0llab0rat0r/py-ipfs-http-client that referenced this pull request Apr 10, 2021
c0llab0rat0r added a commit to c0llab0rat0r/py-ipfs-http-client that referenced this pull request Apr 10, 2021
c0llab0rat0r added a commit to c0llab0rat0r/py-ipfs-http-client that referenced this pull request Apr 14, 2021
c0llab0rat0r added a commit to c0llab0rat0r/py-ipfs-http-client that referenced this pull request Apr 15, 2021
@c0llab0rat0r c0llab0rat0r changed the title Bump min Python version from 3.6.2 to 3.7.2 [wip] Bump min Python version from 3.6.2 to 3.7.2 Apr 15, 2021
@c0llab0rat0r c0llab0rat0r changed the title [wip] Bump min Python version from 3.6.2 to 3.7.2 [hold until 3.6 EOL] Bump min Python version from 3.6.2 to 3.7.2 Apr 15, 2021
ntninja pushed a commit that referenced this pull request Apr 17, 2021
@c0llab0rat0r c0llab0rat0r force-pushed the require-python-37 branch 2 times, most recently from 51d0880 to 9e3ef09 Compare May 17, 2021 03:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants