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

bpo-46124: Update zoneinfo to rely on importlib.resources traversable API. #30190

Merged
merged 3 commits into from
Jan 21, 2022

Conversation

jaraco
Copy link
Member

@jaraco jaraco commented Dec 18, 2021

https://bugs.python.org/issue46124

Automerge-Triggered-By: GH:jaraco

Lib/zoneinfo/_common.py Outdated Show resolved Hide resolved
Lib/zoneinfo/_tzpath.py Outdated Show resolved Hide resolved
Lib/zoneinfo/_tzpath.py Outdated Show resolved Hide resolved
@bedevere-bot
Copy link

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

Lib/zoneinfo/_common.py Outdated Show resolved Hide resolved
@jaraco
Copy link
Member Author

jaraco commented Dec 31, 2021

I have made the requested changes; please review again.

1 similar comment
@jaraco
Copy link
Member Author

jaraco commented Dec 31, 2021

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@pganssle: please review the changes made to this pull request.

@pganssle
Copy link
Member

I don't understand the CI failures. Bedevere seems to think this is a PR against a maintenance branch (it's not) and that it's got no issue number (though it does have one). Maybe rebase and see if that fixes it?

jaraco and others added 3 commits January 21, 2022 09:35
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
Co-authored-by: Paul Ganssle <1377457+pganssle@users.noreply.github.com>
@jaraco jaraco force-pushed the bpo-46124/zoneinfo-resources branch from 07481e5 to dd4c246 Compare January 21, 2022 14:36
@miss-islington
Copy link
Contributor

@jaraco: Status check is done, and it's a success ✅ .

1 similar comment
@miss-islington
Copy link
Contributor

@jaraco: Status check is done, and it's a success ✅ .

@miss-islington
Copy link
Contributor

Sorry, I can't merge this PR. Reason: 4 of 6 required status checks are expected..

1 similar comment
@miss-islington
Copy link
Contributor

Sorry, I can't merge this PR. Reason: 4 of 6 required status checks are expected..

@jaraco
Copy link
Member Author

jaraco commented Jan 21, 2022

Good call. Bedevere seems happy with it now, so I expect it to merge automatically once tests pass.

@miss-islington
Copy link
Contributor

@jaraco: Status check is done, and it's a failure ❌ .

@jaraco jaraco closed this Jan 21, 2022
@jaraco jaraco reopened this Jan 21, 2022
@miss-islington
Copy link
Contributor

@jaraco: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit 00b2b57 into main Jan 21, 2022
@miss-islington miss-islington deleted the bpo-46124/zoneinfo-resources branch January 21, 2022 21:18
@bedevere-bot
Copy link

⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️

Hi! The buildbot ARM64 Windows Non-Debug 3.x has failed when building commit 00b2b57.

What do you need to do:

  1. Don't panic.
  2. Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
  3. Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/730/builds/2799) and take a look at the build logs.
  4. Check if the failure is related to this commit (00b2b57) or if it is a false positive.
  5. If the failure is related to this commit, please, reflect that on the issue and make a new Pull Request with a fix.

You can take a look at the buildbot page here:

https://buildbot.python.org/all/#builders/730/builds/2799

Summary of the results of the build (if available):

== Tests result: ENV CHANGED ==

394 tests OK.

10 slowest tests:

  • test_peg_generator: 4 min 10 sec
  • test_multiprocessing_spawn: 1 min 52 sec
  • test_concurrent_futures: 1 min 34 sec
  • test_mmap: 1 min 25 sec
  • test_asyncio: 1 min 9 sec
  • test_socket: 1 min
  • test_io: 43.4 sec
  • test_distutils: 42.2 sec
  • test_urllib2_localnet: 38.3 sec
  • test_imaplib: 34.2 sec

1 test altered the execution environment:
test_asyncio

37 tests skipped:
test_curses test_dbm_gnu test_dbm_ndbm test_devpoll test_epoll
test_fcntl test_fork1 test_gdb test_grp test_idle test_ioctl
test_kqueue test_multiprocessing_fork
test_multiprocessing_forkserver test_nis test_openpty
test_ossaudiodev test_pipes test_poll test_posix test_pty test_pwd
test_readline test_resource test_spwd test_syslog test_tcl
test_threadsignals test_tix test_tk test_ttk_guionly
test_ttk_textonly test_turtle test_wait3 test_wait4
test_xxtestfuzz test_zipfile64

Total duration: 12 min 5 sec

Click to see traceback logs
Traceback (most recent call last):
  File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\asyncio\proactor_events.py", line 116, in __del__
    self.close()
    ^^^^^^^^^^^^
  File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\asyncio\proactor_events.py", line 108, in close
    self._loop.call_soon(self._call_connection_lost, None)
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\asyncio\base_events.py", line 745, in call_soon
    self._check_closed()
    ^^^^^^^^^^^^^^^^^^^^
  File "C:\Workspace\buildarea\3.x.linaro-win-arm64.nondebug\build\Lib\asyncio\base_events.py", line 506, in _check_closed
    raise RuntimeError('Event loop is closed')
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
RuntimeError: Event loop is closed
k

@jaraco
Copy link
Member Author

jaraco commented Jan 21, 2022

At first blush, that error looks to be a false positive. I can't tell if there's an error or solely a "altered execution environment" failure. It's conceivable that something about test_asyncio is accessing the zoneinfo module and triggering resource issues in how zoneinfo returns open file resources. But I don't see anything of the sort in the tracebacks or test names. Worse, test_asyncio is a package and not a module, so the locus of the failure is diffuse. Still, I confirmed that test_asyncio package makes no reference of zoneinfo, so I'm going to assume the failure is spurious. I'll peek at neighboring buildbot failures to see if I can confirm a failure prior to this commit or a success after.

@jaraco
Copy link
Member Author

jaraco commented Jan 21, 2022

Hmm. Many tests prior to the commit all pass, suggesting this commit was highly unlucky or is in fact implicated. Still awaiting a result for after. I do see that test_proactor_events is implicated in the warnings.

@jaraco
Copy link
Member Author

jaraco commented Jan 21, 2022

Good news. The subsequent build succeeded.

image

So it was just noise. Declaring success.

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.

6 participants