← 返回首页
bpo-1635741: Enhance _datetime error handling by koubaa · Pull Request #23139 · python/cpython · GitHub
Skip to content

Navigation Menu

Toggle navigation
Sign in
Appearance settings
Search or jump to...

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Include my email address so I can be contacted
Cancel Submit feedback

Saved searches

Use saved searches to filter your results more quickly

Cancel Create saved search
Appearance settings
Resetting focus

bpo-1635741: Enhance _datetime error handling#23139

Merged
vstinner merged 4 commits into
python:masterfrom
koubaa:bpo-1635741-datetime
Nov 20, 2020
Merged

bpo-1635741: Enhance _datetime error handling#23139
vstinner merged 4 commits into
python:masterfrom
koubaa:bpo-1635741-datetime

Conversation

koubaa commented Nov 4, 2020 •
edited by bedevere-bot
Loading

Copy link
Copy Markdown
Contributor

This helps prepare for multiphase init (PEP 489)

https://bugs.python.org/issue1635741

koubaa commented Nov 4, 2020

Copy link
Copy Markdown
Contributor Author

@vstinner please review

vstinner reviewed Nov 4, 2020
View reviewed changes

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Please mention the modified module name in the PR title.

I suggest to canghe the PR title to "Enhance _datetime error handling" or "Refactor _datetime error handling", since this change is not directly related to multi-phase init. I'm not sure that we can soon convert this module to the multi-phase init because of the PyCapsule C API.

Comment thread Modules/_datetimemodule.c Outdated Show resolved Hide resolved
Comment thread Modules/_datetimemodule.c Outdated Show resolved Hide resolved
Comment thread Modules/_datetimemodule.c Outdated Show resolved Hide resolved

vstinner commented Nov 4, 2020

Copy link
Copy Markdown
Member

You wrote the wrong bpo number in the PR title.

pganssle commented Nov 4, 2020

Copy link
Copy Markdown
Member

@koubaa Can you provide a TL;DR of why this change is useful or necessary?

vstinner commented Nov 4, 2020

Copy link
Copy Markdown
Member

@koubaa Can you provide a TL;DR of why this change is useful or necessary?

https://bugs.python.org/issue1635741 describes the overall goal: better embed Python in application. One side of the issue is to ensure that an extension module releases all its resources at Python exit.

asvetlov changed the title bpo-1536741: refactoring before multi-phase init bpo-1536741: refactoring datetime before multi-phase init Nov 4, 2020
koubaa changed the title bpo-1536741: refactoring datetime before multi-phase init bpo-1635741: Enhance _datetime error handling Nov 7, 2020
koubaa force-pushed the bpo-1635741-datetime branch from 8910cc2 to 04889ea Compare November 7, 2020 21:37
vstinner reviewed Nov 12, 2020
View reviewed changes
Comment thread Modules/_datetimemodule.c Outdated Show resolved Hide resolved
Comment thread Modules/_datetimemodule.c Outdated Show resolved Hide resolved
Comment thread Modules/_datetimemodule.c Outdated Show resolved Hide resolved
koubaa force-pushed the bpo-1635741-datetime branch from 04889ea to ae49eaf Compare November 15, 2020 17:55
vstinner reviewed Nov 16, 2020
View reviewed changes

vstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality Hide comment

Be careful with preprocessor traps :-( https://gcc.gnu.org/onlinedocs/cpp/Macro-Pitfalls.html

Comment thread Modules/_datetimemodule.c Outdated Show resolved Hide resolved
Comment thread Modules/_datetimemodule.c Show resolved Hide resolved
Comment thread Modules/_datetimemodule.c Outdated Show resolved Hide resolved
Comment thread Modules/_datetimemodule.c Outdated Show resolved Hide resolved
Comment thread Modules/_datetimemodule.c Outdated Show resolved Hide resolved

koubaa commented Nov 20, 2020

Copy link
Copy Markdown
Contributor Author

Be careful with preprocessor traps :-( https://gcc.gnu.org/onlinedocs/cpp/Macro-Pitfalls.html

Thanks again for sharing this, I easily forget them.

vstinner commented Nov 20, 2020

Copy link
Copy Markdown
Member

Thanks again for sharing this, I easily forget them.

Oh, me tee! I got exactly the same issue than you a few days ago :-D

vstinner merged commit 2db8e35 into python:master Nov 20, 2020

vstinner commented Nov 20, 2020

Copy link
Copy Markdown
Member

Thanks, I merged your PR.

I checked manually for refleak using the following test run with "./python -m test (...) -R 3:3":

def test_leak(self): support.run_in_subinterp("import _datetime")

I didn't notice any refleak.

vstinner commented Nov 20, 2020

Copy link
Copy Markdown
Member

Hum, I'm not sure that my manual test is relevant, since _datetime doesn't use multiphase init yet. But it leaks, I don't expect issues with Refleaks buildbots.

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
bpo-1635741: Enhance _datetime error handling (pythonGH-23139)
7a1af5e
kylotan mannequin mentioned this pull request Sep 19, 2022
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

vstinner vstinner left review comments
abalkin Awaiting requested review from abalkin
pganssle Awaiting requested review from pganssle

Assignees

No one assigned

Labels

skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

Footer

© 2026 GitHub, Inc.