-
Notifications
You must be signed in to change notification settings - Fork 1
Add PyConfig_Get() and PyConfig_GetInt() functions #3
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
Comments
I've already reviewed and had my feedback accounted for, so I'm happy with this as-is. String constants are about the best of a bad lot here. They're extensible into the future, and safe to use downlevel (on older versions that don't know about them). About the only possibility here would be to define macros with them, but since we don't support compiling for earlier versions from later versions people wouldn't be able to use the header containing newer names anyway, they'd need to be documented literals anyway. |
I don't feel comfortable looking at this in isolation while the Specific issues I see with this API:
|
A better name might be Possibly we could raise Other than our own tests, I'm not sure what the value of an API to return known names would be? The point of an API like |
Currently, PySys functions are C functions to access the
Ah sure, the exception type can be modified. For example, |
Yes, I know. But from Python code, everything that you might read with this API is coming from the It's a little white lie about where the active configuration is stored that helps keep the mental model consistent between Python code and C code. And from an API design perspective, the mental model is far more important than the actual implementation (I mean it's better to lie if it makes things make more sense to users, especially if it makes things more consistent and one day we might change the implementation to be that way anyway). |
PyInitConfig "Set" API is the PR: python/cpython#110176 You can see the currently proposed API there. I would prefer to work first on the "Get" API since it's simpler, and only later work on the "Set" API.
It's named PyConfig_Get() since names are PyConfig member names. If you dislike it, can you propose another name?
If caller early, before the |
Hi! How can I move this discussion forward? Do you need something? |
I know this is a dimension we haven't discussed yet, but I wonder if adding an argument to get the config from would help the API make more sense? I'm thinking an interpreter state or a thread state, which is obviously what it's using already, but if it's explicit then we don't even have to answer it. The challenge with that is what to do if we need to lock the GIL, and I suspect the answer is that we fail if the tstate isn't current, and then if we figure it out one day (nogil?) then we can relax the constraint. |
And then in that case, maybe |
Hi,
Generating a config dump for an error report, for example.
I get that. But I would like to have a good mental model of what the intended solution is. As proposed, I find the the API surprising and incomplete, and I find it hard to the reasoning behind it. |
Hm, the reason we're adding a new mapping API, and not using the existing |
I expected such API (two functions) to be simple to agree on, but apparently, it's more completed than expected. @encukou now suggests writing a PEP for adding these two functions. I'm already struggling to get a consensus on my PEP 737 – Unify type name formatting. I don't have the bandwidth to fight for too many APIs in parallel. So I prefer to close this issue for now. |
Follow-up: PEP 741: Python Configuration C API. |
Uh oh!
There was an error while loading. Please reload this page.
PR: python/cpython#112609
PyObject* PyConfig_Get(const char *name)
NULL
on error.int PyConfig_GetInt(const char *name, int *value)
*value
and return 0 success.This PR doesn't add these functions to the limited C API.
Voters:
Since I proposed this API and I'm part of the C API Working Group, I will not vote on this decision.
I consider adding these API to the limited C API later, once the PyInitConfig API will added (but this issue is not about PyInitConfig).
These API cover multiple use cases:
Py_VerboseFlag
.PyConfig
structure members (feature not implemetend in the current PR).There is no direct replacement of global configuration variable such as
Py_VerboseFlag
. CallingPyConfig_GetInt("verbose", &verbose)
requires the caller to handle exceptions.PyConfig_GetInt("write_bytecode", &value)
(replacingPy_DontWriteBytecodeFlag
) getssys.dont_write_bytecode
. For example, the function fails if thesys.dont_write_bytecode
attribute has been removed. ThePyConfig_GetInt()
caller is free to ignore the exception, log the exception, or pass the exception to its own caller.Discussion: https://discuss.python.org/t/fr-allow-private-runtime-config-to-enable-extending-without-breaking-the-pyconfig-abi/18004 The use case evolved since August 2022, now an API is also needed for the limited C API.
The text was updated successfully, but these errors were encountered: