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-40077: Convert _queuemodule to use heap types #23136

Merged
merged 5 commits into from Nov 7, 2020

Conversation

erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Nov 3, 2020

@vstinner / @corona10, would you mind reviewing this?

https://bugs.python.org/issue40077

Automerge-Triggered-By: GH:pitrou

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Nov 3, 2020

FYI, rebased onto master, applied _PyType_GetModuleByDef in order to migrate from global to module state.

@rhettinger rhettinger requested a review from pitrou Nov 3, 2020
Modules/_queuemodule.c Show resolved Hide resolved
Modules/_queuemodule.c Show resolved Hide resolved
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Nov 6, 2020

I have made the requested changes; please review again.

@bedevere-bot
Copy link

bedevere-bot commented Nov 6, 2020

Thanks for making the requested changes!

: please review the changes made to this pull request.

@erlend-aasland erlend-aasland requested a review from pitrou Nov 6, 2020
pitrou
pitrou approved these changes Nov 7, 2020
Copy link
Member

@pitrou pitrou left a comment

+1, thank you @erlend-aasland

@pitrou pitrou added the 🤖 automerge PR will be merged once it's been approved and all CI passed label Nov 7, 2020
@miss-islington miss-islington merged commit 01c6aa4 into python:master Nov 7, 2020
3 checks passed
@bedevere-bot
Copy link

bedevere-bot commented Nov 7, 2020

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

Hi! The buildbot s390x RHEL8 LTO + PGO 3.x has failed when building commit 01c6aa4.

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/442/builds/671) and take a look at the build logs.
  4. Check if the failure is related to this commit (01c6aa4) 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/442/builds/671

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

== Tests result: ENV CHANGED ==

409 tests OK.

10 slowest tests:

  • test_concurrent_futures: 2 min 40 sec
  • test_multiprocessing_spawn: 1 min 7 sec
  • test_multiprocessing_forkserver: 1 min 1 sec
  • test_multiprocessing_fork: 52.3 sec
  • test_signal: 47.3 sec
  • test_asyncio: 47.1 sec
  • test_imaplib: 32.2 sec
  • test_pydoc: 31.1 sec
  • test_io: 30.5 sec
  • test_socket: 29.1 sec

1 test altered the execution environment:
test_asyncio

15 tests skipped:
test_devpoll test_gdb test_ioctl test_kqueue test_msilib test_nis
test_ossaudiodev test_startfile test_tix test_tk test_ttk_guionly
test_winconsoleio test_winreg test_winsound test_zipfile64

Total duration: 4 min 54 sec

Click to see traceback logs
Traceback (most recent call last):
  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto-pgo/build/Lib/asyncio/sslproto.py", line 321, in __del__
    self.close()
  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto-pgo/build/Lib/asyncio/sslproto.py", line 316, in close
    self._ssl_protocol._start_shutdown()
  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto-pgo/build/Lib/asyncio/sslproto.py", line 590, in _start_shutdown
    self._abort()
  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto-pgo/build/Lib/asyncio/sslproto.py", line 731, in _abort
    self._transport.abort()
  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto-pgo/build/Lib/asyncio/selector_events.py", line 680, in abort
    self._force_close(None)
  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto-pgo/build/Lib/asyncio/selector_events.py", line 731, in _force_close
    self._loop.call_soon(self._call_connection_lost, exc)
  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto-pgo/build/Lib/asyncio/base_events.py", line 746, in call_soon
    self._check_closed()
  File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel8-z.lto-pgo/build/Lib/asyncio/base_events.py", line 510, in _check_closed
    raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed

@erlend-aasland erlend-aasland deleted the bpo-40077/queue branch Nov 7, 2020
@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Nov 7, 2020

Thanks for reviewing, @pitrou !

erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Dec 28, 2020
The typo did no damage, but it looks suspicious and confusing.
Introduced by pythonGH-23136.
miss-islington pushed a commit that referenced this pull request Dec 28, 2020
The typo did no damage, but it looks suspicious and confusing.
Introduced by GH-23136.

Skip news.

Automerge-Triggered-By: GH:pitrou
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
The typo did no damage, but it looks suspicious and confusing.
Introduced by pythonGH-23136.

Skip news.

Automerge-Triggered-By: GH:pitrou
@vstinner
Copy link
Member

vstinner commented Apr 4, 2021

@pitrou: You merged the commit message with the sentence:

@vstinner / @corona10, would you mind reviewing this?

And I'm now getting emails from cpython forks, like this commit:

JelleZijlstra@01c6aa4

@pitrou
Copy link
Member

pitrou commented Apr 4, 2021

Ah, sorry. I get this sometimes when my name is mentioned too :-)

@vstinner
Copy link
Member

vstinner commented Apr 6, 2021

Ah, sorry. I get this sometimes when my name is mentioned too :-)

Well, the "automerge" label causes a similar feature and it's part of our workflow. I'm not sure how to ignore such emails. @pganssle has such issue: https://twitter.com/pganssle/status/1379161385863643155

@erlend-aasland
Copy link
Contributor Author

erlend-aasland commented Apr 6, 2021

It's partly my fault; I cc'd you in the PR text instead of in a separate comment/post. After this incident, I've made sure I don't mention anyone in the PR text. Maybe it could be mentioned in the Dev Guide? Maybe we could make the CI prevent it by rewriting the commit message (if needed) upon merging.

@vstinner
Copy link
Member

vstinner commented Apr 6, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 automerge PR will be merged once it's been approved and all CI passed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants