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.

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

Jump to bottom

Bump to Python 3.9 attempt 2. #3623

Closed
wants to merge 26 commits into from
Closed

Bump to Python 3.9 attempt 2. #3623

wants to merge 26 commits into from

Conversation

moodyjon
Copy link
Contributor

Based on work by @eukreign in branch py39.

I am still not sure what issues in the tests are truly due to Python upgrade vs. pre-existing timing issues. I have some hacks in there adding asyncio.sleep().

Other risky areas are related to Python 3.8 changing the base class of asyncio.CancelledError from Exception to BaseException. I fixed some places that assumed the old base class, but there may be more. See 8bb7805 and 528a37a

I am excited about upgrading to 3.9 because it lets me run more things natively on MacOS arm64.

Change a369228 should be temporary, and lets me repeat tests manually to assess reliability.

@moodyjon moodyjon marked this pull request as draft June 19, 2022 12:45
@moodyjon moodyjon mentioned this pull request Jun 19, 2022
Merged
@moodyjon
Copy link
Contributor Author

Putting this in draft status, pending fix for some (probably) unrelated test failures:

  1. (pending fixes) Fixes for intermittent test failures: test_exchange_rate_manager(), test_basic_claim_search() #3624
  2. (hub? fixes needed) lbry-sdk suite: test_es_sync_utility: AssertionError: 217 != 218 hub#55

@eukreign
Copy link
Member

eukreign commented Jul 8, 2022

@moodyjon I'm really excited about your work on this! Really looking forward to be able to upgrade to 3.9. I will review all of this as soon as you get all of the tests passing and give me a heads up.

@eukreign eukreign assigned moodyjon and unassigned eukreign Jul 8, 2022
@coveralls
Copy link

coveralls commented Jul 8, 2022 •
edited
Loading

Coverage Status

Coverage increased (+0.4%) to 58.134% when pulling fcffe1b on moodyjon:py39_2 into 01cd95f on lbryio:master.

@robd003 robd003 mentioned this pull request Jul 13, 2022
Open
@moodyjon moodyjon mentioned this pull request Jul 26, 2022
Merged
eukreign and others added 16 commits August 11, 2022 14:58
@eukreign @moodyjon
try building 3.9
56bae9f
@eukreign @moodyjon
update plyvel
1c8c84e
@eukreign @moodyjon
passing loop to asyncio functions is deprecated
ec6b6fb
@eukreign @moodyjon
shuffle() needs custom random, removed loop from Event()/Queue()
b2ef7f6
@eukreign @moodyjon
pylint disable shuffle() arg check
2031f65
@eukreign @moodyjon
upgrade cryptography
51a345c
@moodyjon
Bump pylint version. Old pylint did not find standard library stuff o…
58716e4 
…n 3.9.12.
@moodyjon
Fix and enable lint no-self-use & try-except-raise.
1079908
@moodyjon
Fix Transport, DatagramTransport mockup issues.
7f83a0c
@moodyjon
Make stop(), stop_tasks() consistently async routines, and have stop_…
d927b9e 
…tasks()

wait for file_output_task completion. This fixes a problem with
test_download_stop_resume_delete.
@moodyjon
CancelledError derives from BaseException in Python >= 3.8. The signi…
1b229a5 
…ficant functional

change here is in upload_to_reflector(). Unit tests in TestReflector were failing.
Deal with lint related to CancelledError cleanup.
@moodyjon
Tweak aiohttp version to be compatible with hub repository.
c223bcb
@moodyjon
Make FileManager.stop() async because SourceManager.stop() is now async.
3bc566f
@moodyjon
Try ubuntu-20.04 to resolve missing libffi.so.7 issue.
d134cbf
@moodyjon
Try separate file_manager.stop() and start() calls to better
aba53c8 
control order of events in test.
While file_manager is stopped, we get no response to file_list().
@moodyjon
Another place generalized to Exception or asyncio.CancelledError.
82cf809
moodyjon added 10 commits August 11, 2022 15:06
@moodyjon
Sleep in test_trending(). Suppress CancelledError exception.
412279d
@moodyjon
TEMP: Add workflow_dispatch.
7fefcdd
@moodyjon
Wait 3s for height == 206 in test_blockchain_reorganization.py.
a271114
@moodyjon
Try to suppress asyncio.CancelledError in a different way in test_str…
bc021c4 
…eaming.py.
@moodyjon
More cleanup of deprecated loop= arguments for asyncio.Queue,Event,Lock.
a4f9a7e
@moodyjon
Undo loop= argument elimination for TorrentHandle, Torrent.
82f652c
@moodyjon
Sleep in test_delay_takeover_with_update_then_update_to_lower_before_…
6dd3f4b 
…takeover().
@moodyjon
More cleanup (tests/...) of deprecated loop= arguments for asyncio.Qu…
44ccb4c 
…eue, asyncio.sleep.
@moodyjon
Wait for block 206 for a short period in test_resolve_command.py
9dd27a5
@moodyjon
Work on some DeprecationWarnings: The explicit passing of coroutine o…
fcffe1b 
…bjects to asyncio.wait() is deprecated since Python 3.8.
@moodyjon
Copy link
Contributor Author

I have synced up to head-of-line, and am running tests to asses where the next pain point is in the tests. Most of this is going on in moodyjon/lbry-sdk actions using the manual trigger.

As test_setting_stream_fields against (go) herald has been disabled, that is no longer a deterministic failure.

@moodyjon
Copy link
Contributor Author

Out of 20 runs, I got 5 failures. test_connection_drop_still_receives_events_after_reconnected was seen 2 times, the others 1 time.

py39_2 ci failures.txt

test_file_save_stop_before_finished_streaming_only_wait_for_start is one I had been modifying heavily, though I don't really know what to make of the way it failed. A bunch of debugging info was dumped out and coverage run .... integration.datanetwork exited abruptly with SIGSEGV during the test.

180
ERROR: InvocationError for command /home/runner/work/lbry-sdk/lbry-sdk/.tox/datanetwork/bin/coverage run -p --source=/home/runner/work/lbry-sdk/lbry-sdk/.tox/datanetwork/lib/python3.9/site-packages/lbry -m unittest discover -vv integration.datanetwork (exited with code -11 (SIGSEGV)) (exited with code -11)

I think I've seen the SIGSEGV before in other workspaces. Any insight into whether Python code can do this? or could it be the coverage tool combined with some feature of the code?

@moodyjon moodyjon mentioned this pull request Nov 21, 2022
Merged
@moodyjon
Copy link
Contributor Author

Closing in favor of #3711

@moodyjon moodyjon closed this Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@moodyjon moodyjon

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@moodyjon @eukreign @coveralls