Rust API for pyinitconfig - #6173
Conversation
2f0308c to
f15f4fc
Compare
|
According to codecov almost none of the API surface is tested. Can you please make sure there's coverage for all the |
57b35c8 to
f622003
Compare
Merging this PR will not alter performance
Comparing 
Footnotes
|
|
@codspeedbot fix this regression |
|
Maintainers (@ngoldbaum), is test coverage OK. Do we need a test case which deliberately causes initialization to fail so the error handling code is covered? |
c8f66ec to
99b41b0
Compare
141a505 to
7931138
Compare
davidhewitt
left a comment
davidhewitt
left a comment
There was a problem hiding this comment.
Thanks, some various thoughts and suggestions. Sorry it took a long time to get around to this one.
| let result = | ||
| unsafe { PyInitConfig_GetExitCode(self.0.as_ptr(), &raw mut exitcode) }; |
There was a problem hiding this comment.
If initialization fails, is it safe to try again? (Either with the same config object or a new one.)
I assume that the answer is probably yes we can assume the main interpreter has fully finalized and we can initialize a new one? I wonder if we can test for that somehow.
There was a problem hiding this comment.
It's not safe to try again with the same object because modules in the init tab may be invalidated. That is why the initialize function consumes the config object instead of taking it by reference.
I think it's fine to use a new config or no config.
I'm not sure about testing. I suppose we could try to attach a thread state and assert that it fails. Does that seem right?
| config | ||
| .set_str(c"prefix", c"/path/that/does/not/exit") | ||
| .unwrap(); | ||
| config.initialize().unwrap_err(); |
There was a problem hiding this comment.
As per other comment I wonder if it's possible to try again here.
There was a problem hiding this comment.
Is there something you specifically want to be different in this test case?
bbb77fe to
2c43390
Compare
913ded1 to
19a4a9b
Compare
5ca5f0a to
5c01d67
Compare
4 participants
Adds a rust API to initialize the embedded interpreter with configuration.
Closes #6153