gh-143698: correctly check scheduler and setpgroup values for os.posix_spawn[p]

[picnixz]

• Issue: SystemError when passing invalid argument to posix_spawn #143698

[johnslavik]

The documentation to me is a bit subtle about providing this parameter:

The scheduler argument must be a tuple containing the (optional) scheduler policy and an instance of sched_param with the scheduler parameters. (...) A value of None in the place of the scheduler policy indicates that is not being provided. (...)

Do I understand correctly that:

• scheduler=None is just an illustrative figure in the documentation
• one can either (a) only to provide as a 2-element tuple or (b) not provide it at all
• consequently, one cannot explicitly set scheduler=None in the code, even though the documentation uses scheduler=None as an illustrative figure

?

[picnixz]

scheduler=None is just an illustrative figure in the documentation

Yes, None is only valid for the scheduler policy (so first item of the tuple) not the scheduler itself. I think we can either have an overload for the signature with scheduler omitted, or I can allow scheduler=None explicitly so that the doc is fine.

[johnslavik]

or I can allow scheduler=None explicitly so that the doc is fine.

I think it would be better, yeah 👍
Unless there are some downsides to supporting an explicit scheduler=None?

[picnixz]

Unless there are some downsides to supporting an explicit scheduler=None?

Mmh. I don't think so (?) at least I don't have an example

[donbarbos]

I'd like to note that if you pass None to setpgroup, a TypeError is also thrown (this is a common behavior, it might be worth specifying a different default value in the documentation to avoid user confusion)

[picnixz]

Hum. I don't know if I want to split this PR. I think it's better, but it won't help for reverts because reverting one change would anyway create a conflict with another.

@vstinner Do you want me to split this PR just for commit history being a bit clearer?

[picnixz]

Oh I forgot about those inspect tests. Ok, I'll do two PRs.

[vstinner]

LGTM

[vstinner]

Oh I forgot about those inspect tests. Ok, I'll do two PRs.

Right, test_inspect is failing on CIs. Why do you want to make two PRs? You can update test_inspect in this PR, no?

[picnixz]

That is what I thought but since it has both an additionnal feature (that is the setpgroup stuff) I maybe wanted a separate PR (as this would change more than one thing). I thought about first fixing the crash and then allowing explicit None.

[miss-islington-app]

Thanks @picnixz for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14.
🐍🍒⛏🤖

[miss-islington-app]

Sorry, @picnixz, I could not cleanly backport this to 3.14 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 347fc438cf903c1d7fa5063464ae2e93c11b2232 3.14

[miss-islington-app]

Sorry, @picnixz, I could not cleanly backport this to 3.13 due to a conflict.
Please backport using cherry_picker on command line.

cherry_picker 347fc438cf903c1d7fa5063464ae2e93c11b2232 3.13

[bedevere-app]

GH-145073 is a backport of this pull request to the 3.14 branch.

[bedevere-app]

GH-145074 is a backport of this pull request to the 3.13 branch.