gh-108277: Add wrapper for timerfd_create, timerfd_settime, and timerfd_gettime to os module by m-tmatma · Pull Request #108382 · python/cpython

m-tmatma · 2023-08-23T21:54:34Z

Add wrapper for timerfd system calls.

Issue: Add wrapper for timerfd_create, timerfd_settime, and timerfd_gettime to os module #108277

Note: local test

python3 Tools/clinic/clinic.py -f Modules/posixmodule.c && \
autoconf -i && \
./configure && \
make regen-configure && \
make -j4 regen-all && \
make regen-stdlib-module-names && \
make test TESTOPTS="-v test_os" && \
make -C Doc venv && make -C Doc html

📚 Documentation preview 📚: https://cpython-previews--108382.org.readthedocs.build/

serhiy-storchaka

In general LGTM, but we must first discuss what exactly we want to get.

serhiy-storchaka · 2023-08-28T05:55:17Z

Please add also an entry in the What's New file.

…ttime, and timerfd_gettime to 'os' module.

erlend-aasland · 2023-08-28T09:30:05Z

Just a heads-up, @m-tmatma: please don't force-push; instead, git merge --no-ff main. It makes for a better review experience, since both CI runs and comments can be tied to specific commits.

Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

m-tmatma · 2023-08-28T10:09:11Z

@erlend-aasland

Just a heads-up, @m-tmatma: please don't force-push; instead, git merge --no-ff main. It makes for a better review experience, since both CI runs and comments can be tied to specific commits.

Sorry, I thought force-push is a manner to contribute.

m-tmatma · 2023-08-28T10:26:32Z

@serhiy-storchaka

Please add also an entry in the What's New file.

Added.

vstinner

LGTM. Thanks for all updates!

IMO using _PyTime_t API and passing the rounding method from the time module is the right way to handle rounding.

vstinner · 2023-10-07T17:33:27Z

I merged your PR, thanks @m-tmatma for your great work!

Some remarks:

The PR got reviewed by multiple core devs.
IMO os.rst doc should have a link to the HOWTO: please propose a follow-up PR for that.
I like the API with only 1 positional-only parameter, and then keyword-only arguments: we need more APIs like that :-)
I'm scared by the size of the change, the size of the API, but I'm reassured by the length of the documentation and the length of the tests. Thanks for writing all of that!
I also merged this PR because it has too many comments, too many commits. It became really hard to track it :-) I merge it to have a "starting point". Later, we can obviously enhance doc/implementation/tests/whatever.

bedevere-bot · 2023-10-07T17:39:50Z

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

Hi! The buildbot AMD64 RHEL7 3.x has failed when building commit de2a403.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/15/builds/5889) and take a look at the build logs.
Check if the failure is related to this commit (de2a403) or if it is a false positive.
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/15/builds/5889

Failed tests:

test_os

Failed subtests:

test_timerfd_negative - test.test_os.TimerfdTests.test_timerfd_negative

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

==

Click to see traceback logs

Traceback (most recent call last):
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-x86_64/build/Lib/test/test_os.py", line 3987, in test_timerfd_negativefor flags in (0, os.TFD_TIMER_ABSTIME, os.TFD_TIMER_ABSTIME|os.TFD_TIMER_CANCEL_ON_SET):
^^^^^^^^^^^^^^^^^^^^^^^^^^AttributeError: module 'os' has no attribute 'TFD_TIMER_CANCEL_ON_SET'

bedevere-bot · 2023-10-07T17:43:54Z

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

Hi! The buildbot s390x RHEL7 3.x has failed when building commit de2a403.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/179/builds/5673) and take a look at the build logs.
Check if the failure is related to this commit (de2a403) or if it is a false positive.
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/179/builds/5673

Failed tests:

test_os

Failed subtests:

test_timerfd_negative - test.test_os.TimerfdTests.test_timerfd_negative

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

==

Click to see traceback logs

Traceback (most recent call last):
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z/build/Lib/test/test_os.py", line 3987, in test_timerfd_negativefor flags in (0, os.TFD_TIMER_ABSTIME, os.TFD_TIMER_ABSTIME|os.TFD_TIMER_CANCEL_ON_SET):
^^^^^^^^^^^^^^^^^^^^^^^^^^AttributeError: module 'os' has no attribute 'TFD_TIMER_CANCEL_ON_SET'

bedevere-bot · 2023-10-07T17:55:36Z

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

Hi! The buildbot s390x RHEL7 LTO + PGO 3.x has failed when building commit de2a403.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/244/builds/5632) and take a look at the build logs.
Check if the failure is related to this commit (de2a403) or if it is a false positive.
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/244/builds/5632

Failed tests:

test_os

Failed subtests:

test_timerfd_negative - test.test_os.TimerfdTests.test_timerfd_negative

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

==

Click to see traceback logs

Traceback (most recent call last):
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto-pgo/build/Lib/test/test_os.py", line 3987, in test_timerfd_negativefor flags in (0, os.TFD_TIMER_ABSTIME, os.TFD_TIMER_ABSTIME|os.TFD_TIMER_CANCEL_ON_SET):
^^^^^^^^^^^^^^^^^^^^^^^^^^AttributeError: module 'os' has no attribute 'TFD_TIMER_CANCEL_ON_SET'

bedevere-bot · 2023-10-07T18:05:52Z

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

Hi! The buildbot s390x RHEL7 LTO 3.x has failed when building commit de2a403.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/402/builds/5581) and take a look at the build logs.
Check if the failure is related to this commit (de2a403) or if it is a false positive.
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/402/builds/5581

Failed tests:

test_os

Failed subtests:

test_timerfd_negative - test.test_os.TimerfdTests.test_timerfd_negative

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

==

Click to see traceback logs

Traceback (most recent call last):
File "/home/dje/cpython-buildarea/3.x.edelsohn-rhel-z.lto/build/Lib/test/test_os.py", line 3987, in test_timerfd_negativefor flags in (0, os.TFD_TIMER_ABSTIME, os.TFD_TIMER_ABSTIME|os.TFD_TIMER_CANCEL_ON_SET):
^^^^^^^^^^^^^^^^^^^^^^^^^^AttributeError: module 'os' has no attribute 'TFD_TIMER_CANCEL_ON_SET'

bedevere-bot · 2023-10-07T18:13:05Z

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

Hi! The buildbot AMD64 Ubuntu Shared 3.x has failed when building commit de2a403.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/506/builds/6080) and take a look at the build logs.
Check if the failure is related to this commit (de2a403) or if it is a false positive.
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/506/builds/6080

Failed tests:

test_os

Failed subtests:

test_timerfd_ns_TFD_TIMER_ABSTIME - test.test_os.TimerfdTests.test_timerfd_ns_TFD_TIMER_ABSTIME

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

==

Click to see traceback logs

Traceback (most recent call last):
File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test_os.py", line 4211, in test_timerfd_ns_TFD_TIMER_ABSTIMEself.assertGreater(t, offset_ns)
AssertionError: 999978637 not greater than 1000000000

bedevere-bot · 2023-10-07T18:17:18Z

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

Hi! The buildbot s390x Fedora Clang 3.x has failed when building commit de2a403.

What do you need to do:

Don't panic.
Check the buildbot page in the devguide if you don't know what the buildbots are or how they work.
Go to the page of the buildbot that failed (https://buildbot.python.org/all/#builders/3/builds/4767) and take a look at the build logs.
Check if the failure is related to this commit (de2a403) or if it is a false positive.
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/3/builds/4767

Failed tests:

test_os

Failed subtests:

test_timerfd_ns_TFD_TIMER_ABSTIME - test.test_os.TimerfdTests.test_timerfd_ns_TFD_TIMER_ABSTIME
test_timerfd_TFD_TIMER_ABSTIME - test.test_os.TimerfdTests.test_timerfd_TFD_TIMER_ABSTIME

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

==

Click to see traceback logs

Traceback (most recent call last):
File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/test/test_os.py", line 4211, in test_timerfd_ns_TFD_TIMER_ABSTIMEself.assertGreater(t, offset_ns)
AssertionError: 999998128 not greater than 1000000000
Traceback (most recent call last):
File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/test/test_os.py", line 4058, in test_timerfd_TFD_TIMER_ABSTIMEself.assertGreater(t, offset)
AssertionError: 0.9999921415001154 not greater than 1

m-tmatma · 2023-10-07T20:54:51Z

@vstinner
Thank you for merging.

vstinner · 2023-10-07T21:02:17Z

@m-tmatma: Now you can enjoy the great pleasure of having to fix test failures :-) Look at previous comments from buildbots.

======================================================================
FAIL: test_timerfd_TFD_TIMER_ABSTIME (test.test_os.TimerfdTests.test_timerfd_TFD_TIMER_ABSTIME)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/test/test_os.py", line 4058, in test_timerfd_TFD_TIMER_ABSTIME
self.assertGreater(t, offset)
AssertionError: 0.9999921415001154 not greater than 1
======================================================================
FAIL: test_timerfd_ns_TFD_TIMER_ABSTIME (test.test_os.TimerfdTests.test_timerfd_ns_TFD_TIMER_ABSTIME)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/dje/cpython-buildarea/3.x.edelsohn-fedora-z.clang/build/Lib/test/test_os.py", line 4211, in test_timerfd_ns_TFD_TIMER_ABSTIME
self.assertGreater(t, offset_ns)
AssertionError: 999998128 not greater than 1000000000

m-tmatma · 2023-10-07T21:29:48Z

@vstinner
sorry, but I'm going to travel. so it takes a week to start fixing.

m-tmatma · 2023-10-07T21:42:04Z

@vstinner
I didn't notice these failures until the PR
was merged.

Some tests are only run only on main branch?

There were some difficulties when I had developed this feature because actions was not enabled on feature branches.

I think all actions jobs should be run on feature branches or at least PRs to prevent the case on the PR.

vstinner · 2023-10-08T00:52:11Z

@vstinner: I didn't notice these failures until the PR was merged.

It's because buildbots are not run before a PR is actually merged, until someone asks explicitly to run buildbots on a PR, which wasn't done.

It's ok. I may have a look at these failures and try to fix them.

vstinner · 2023-10-08T00:58:17Z

On AMD64 RHEL7 3.x, test_timerfd_negative() fails because os.TFD_TIMER_CANCEL_ON_SET doesn't exist. test.pythoninfo:

os.uname: posix.uname_result(sysname='Linux', nodename='python-builder-rhel7.osci.io', release='3.10.0-1160.95.1.el7.x86_64', version='#1 SMP Fri Jun 23 08:44:55 EDT 2023', machine='x86_64')
platform.architecture: 64bit ELF
platform.freedesktop_os_release[ID]: rhel
platform.freedesktop_os_release[NAME]: Red Hat Enterprise Linux Server
platform.freedesktop_os_release[VARIANT_ID]: server
platform.freedesktop_os_release[VERSION]: 7.9 (Maipo)
platform.freedesktop_os_release[VERSION_ID]: 7.9
platform.libc_ver: glibc 2.17
platform.platform: Linux-3.10.0-1160.95.1.el7.x86_64-x86_64-with-glibc2.17

vstinner · 2023-10-08T01:06:08Z

AMD64 Ubuntu Shared 3.x: https://buildbot.python.org/all/#/builders/506/builds/6080

FAIL: test_timerfd_ns_TFD_TIMER_ABSTIME (test.test_os.TimerfdTests.test_timerfd_ns_TFD_TIMER_ABSTIME)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/srv/buildbot/buildarea/3.x.bolen-ubuntu/build/Lib/test/test_os.py", line 4211, in test_timerfd_ns_TFD_TIMER_ABSTIME
self.assertGreater(t, offset_ns)
AssertionError: 999978637 not greater than 1000000000

vstinner · 2023-10-08T01:07:40Z

I wrote PR #110515 to fix tests.

m-tmatma · 2023-10-08T02:36:27Z

@vstinner
Thank you for the PR.

vstinner · 2023-10-08T21:45:34Z

It seems like buildbots are happy, good. Refleaks will run with the fix this night.

Add wrapper for timerfd_create, timerfd_settime, and timerfd_gettime to os module. Co-authored-by: Serhiy Storchaka <storchaka@gmail.com> Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com> Co-authored-by: Erlend E. Aasland <erlend.aasland@protonmail.com> Co-authored-by: Victor Stinner <vstinner@python.org>

m-tmatma requested review from corona10 and erlend-aasland as code owners August 23, 2023 21:54

This comment was marked as resolved.
Sign in to view

bedevere-bot mentioned this pull request Aug 23, 2023
Add wrapper for timerfd_create, timerfd_settime, and timerfd_gettime to os module #108277
Closed

This comment was marked as outdated.
Sign in to view

bedevere-bot added the awaiting review label Aug 23, 2023

m-tmatma force-pushed the feature/os.timerfd_xxx branch from dbf9c7a to 5ecbf20Compare August 23, 2023 21:55

This comment was marked as outdated.
Sign in to view

m-tmatma force-pushed the feature/os.timerfd_xxx branch from 5ecbf20 to c192e22Compare August 23, 2023 21:57

This comment was marked as outdated.
Sign in to view

m-tmatma force-pushed the feature/os.timerfd_xxx branch 5 times, most recently from 61a9908 to ebd989fCompare August 26, 2023 09:34

serhiy-storchaka reviewed Aug 28, 2023
View reviewed changes

Comment threadDoc/library/os.rst Outdated
Comment threadModules/posixmodule.c

erlend-aasland reviewed Aug 28, 2023
View reviewed changes

Comment threadModules/posixmodule.c Outdated

m-tmatmaand others added 3 commits August 28, 2023 18:28

pythongh-108277: Add wrapper functions for timerfd_create, timerfd_se…
0f8aaf5
…ttime, and timerfd_gettime to 'os' module.

📜🤖 Added by blurb_it.
208fd58

fix lint error
37f70cd

m-tmatma force-pushed the feature/os.timerfd_xxx branch from ebd989f to 37f70cdCompare August 28, 2023 09:28

fix typo in doc
6f63cf7

m-tmatmaand others added 3 commits August 28, 2023 18:31

Update Modules/posixmodule.c
0f47920
Co-authored-by: Serhiy Storchaka <storchaka@gmail.com>

re-run Tools/clinic/clinic.py
03f319d

define HAVE_TIMERFD_API only
b8e39f2

update What's New file
44b3ac7

vstinner reviewed Oct 5, 2023
View reviewed changes

Comment threadLib/test/test_os.py

vstinner reviewed Oct 5, 2023
View reviewed changes

Comment threadPython/pytime.c Outdated

m-tmatma added 5 commits October 7, 2023 08:25

Merge branch 'main' into feature/os.timerfd_xxx
ccaf48d

move common code to a utility function
b334a73

add rounding parameter to _PyTime_FromSecondsDouble
265ea56

Merge branch 'main' into feature/os.timerfd_xxx
945ef3c

replace HAVE_TIMERFD_API with HAVE_TIMERFD_CREATE
6eefda5

vstinner approved these changes Oct 7, 2023
View reviewed changes

vstinner merged commit de2a403 into python:mainOct 7, 2023

bedevere-appBot removed the awaiting core review label Oct 7, 2023

m-tmatma deleted the feature/os.timerfd_xxx branch October 7, 2023 20:55

vstinner mentioned this pull request Oct 11, 2023
test_os.TimerfdTests fails on FreeBSD 14 and 15: missing select.epoll() #110697
Closed

Uh oh!

Uh oh!

Conversation

m-tmatma commented Aug 23, 2023• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

This comment was marked as outdated.

This comment was marked as outdated.

This comment was marked as outdated.

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

serhiy-storchaka commented Aug 28, 2023

Uh oh!

Uh oh!

erlend-aasland commented Aug 28, 2023

Uh oh!

m-tmatma commented Aug 28, 2023

Uh oh!

m-tmatma commented Aug 28, 2023

Uh oh!

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

vstinner commented Oct 7, 2023

Uh oh!

bedevere-bot commented Oct 7, 2023

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

Uh oh!

bedevere-bot commented Oct 7, 2023

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

Uh oh!

bedevere-bot commented Oct 7, 2023

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

Uh oh!

bedevere-bot commented Oct 7, 2023

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

Uh oh!

bedevere-bot commented Oct 7, 2023

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

Uh oh!

bedevere-bot commented Oct 7, 2023

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

Uh oh!

m-tmatma commented Oct 7, 2023

Uh oh!

vstinner commented Oct 7, 2023

Uh oh!

m-tmatma commented Oct 7, 2023

Uh oh!

m-tmatma commented Oct 7, 2023

Uh oh!

vstinner commented Oct 8, 2023

Uh oh!

vstinner commented Oct 8, 2023

Uh oh!

vstinner commented Oct 8, 2023

Uh oh!

vstinner commented Oct 8, 2023

Uh oh!

m-tmatma commented Oct 8, 2023

Uh oh!

vstinner commented Oct 8, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

m-tmatma commented Aug 23, 2023•
edited
Loading