gh-152433: Windows: _ssl module: improve UWP compatibility#152791
Open
thexai wants to merge 1 commit into
Open
gh-152433: Windows: _ssl module: improve UWP compatibility#152791thexai wants to merge 1 commit into
_ssl module: improve UWP compatibility#152791thexai wants to merge 1 commit into
Conversation
picnixz
reviewed
Jul 1, 2026
picnixz
left a comment
picnixz
left a comment
Member
There was a problem hiding this comment.
- Please use
if defined()andelif defined(MS_WINDOWS) && ...andelsefor the fallback branch. - Can you determine why we are only raising a NotImplementedError on MS DEBUG builds for those functions? it's a separate issue but I think we should also raise unconditionally (or does it mean that those functions are available on regular MS builds only in release builds?) If there is something worth investigating, you can open a separate issue for that.
| PyErr_SetString(PyExc_NotImplementedError, "load_dh_params: unavailable on UWP build"); | ||
| return NULL; | ||
| #else | ||
| #if defined(MS_WINDOWS) && defined(Py_DEBUG) |
Member
There was a problem hiding this comment.
The fact that we raise only on DEBUG MS builds is really weird here. Likewise, I would rather prefer an elif branch instead of a ifdef/else + nested.
thexai
force-pushed
the
uwp-_ssl
branch
2 times, most recently
from
834b4c3 to
4db23b3
Compare
picnixz
reviewed
Jul 3, 2026
picnixz
approved these changes
Jul 3, 2026
picnixz
reviewed
Jul 3, 2026
| @@ -0,0 +1 @@ | |||
| ``_ssl`` module: improve UWP compatibility. | |||
Member
There was a problem hiding this comment.
Suggested change
| ``_ssl`` module: improve UWP compatibility. | |
| :mod:`ssl`: improve UWP build compatibility. |
I think we can advertise it like that. The problem was the build right?
picnixz
approved these changes
Jul 3, 2026
picnixz
left a comment
picnixz
left a comment
Member
There was a problem hiding this comment.
This PR was small enough and changes were easy to spot, but in the future, please avoid force pushing once someone started reviewing your PR as it makes incremental review much harder. We squash merge at the end and change the commit messages ourselves when necessary.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Windows:
_sslmodule: improve UWP compatibility.