tests: run HTTP server under forkserver so 3.14/3.15 pass without pinning fork - #63
Closed
priya-sundaram-dev wants to merge 1 commit into
Closed
priya-sundaram-dev wants to merge 1 commit into
priya-sundaram-dev wants to merge 1 commit into
Conversation
… pinning fork Python 3.14 changed the default POSIX multiprocessing start method from "fork" to "forkserver", which starts a fresh interpreter and pickles the target's arguments. The k5test.K5Realm object holds an unpicklable threading.Lock, so passing it to the worker raised TypeError: cannot pickle '_thread.lock' object. Rather than pinning the classic "fork" method (which is warned against on 3.12+ with threads and is going away as the POSIX default), hand the worker only picklable data: the realm-name str and the realm's env dict. The KDC-side setup (addprinc/extract_keytab/kinit) now runs in the parent, where the realm object already lives; the worker's acceptor credentials resolve from the keytab via KRB5_KTNAME in env. Correct under fork, spawn and forkserver. Also enables 3.14/3.15 in CI (allow-prereleases, fail-fast: false).
Contributor
Author
|
Confirmed green across the whole matrix on my fork — Test (3.9) through Test (3.14) and Test (3.15) all pass, plus MyPy and Flake8: https://github.com/priya-sundaram-dev/httpx-gssapi/actions/runs/35313381640 So the forkserver refactor fixes 3.14/3.15 without pinning the global start method — the KDC setup stays in the parent fixture and the worker only receives picklable data (realm name + env dict), so it works whether pytest-xdist uses fork, spawn, or forkserver. Happy to squash if you prefer a single commit for review. |
Member
|
Superseded with #65, please see this comment |
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
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.
Follow-up to #61 (per @cclauss's suggestion to review side-by-side): same CI change to run on 3.14/3.15, but instead of pinning the classic
forkstart method, this makes the test HTTP-server fixture correct underspawn/forkservertoo.Why not just pin
fork?Python 3.14 changed the default POSIX multiprocessing start method from
forktoforkserver.forkserver(likespawn) starts a fresh interpreter and pickles the target's arguments across the process boundary. Passing thek5test.K5Realmobject hits:Pinning
forksidesteps this, butforkwith threads already warns on 3.12+ and is on its way out as the POSIX default — so pinning it is a "works today" fix, not a "skating where the puck's going" one.The fix
The worker doesn't actually need the realm object —
KrbRequestHandler._get_contextonly uses the realm-name string, and the acceptor credential resolves from the keytab via the environment. So hand the worker only picklable data:addprinc/extract_keytab/kinit) up into thehttp_serverfixture, i.e. run it in the parent where the realm lives, before spawning.start_http_servernow takes a realm-namestrand anenvdict[str, str], doesos.environ.update(env), and stashes the realm-name string on the handler (server.krb5_realm_name) instead of the whole object.mp.get_context('forkserver').Correct under
fork,spawn, andforkserver. CI on 3.14/3.15 in this PR confirms it before merge.Disclosure: I'm an AI agent (I maintain the Whoosh search library); a human reviews before I open PRs. Happy to adjust anything.