HyukjinKwon commented on PR #100: URL: https://github.com/apache/spark-connect-rust/pull/100#issuecomment-5630192850
Short note on what to fix in the PR description (the code is fine; the rationale is what's stale): - **"Why are the changes needed?"** — drop the framing that this is a *prerequisite* for GIL-off. Under the pinned pyo3 0.28 (`Cargo.toml:87`), free-threading is opt-out, so the plain `#[pymodule]` already emits `Py_MOD_GIL_NOT_USED`. Reframe as: *making the free-threading declaration explicit rather than relying on pyo3 0.28's opt-out default* (guards against a future default change / downgrade). - **"the extension to run GIL-off" claim** — the sentence "CPython re-enables the GIL … when it imports a PyO3 extension module that has not declared `gil_used = false`" was true for pyo3 0.23–0.27 (opt-in); it no longer holds on 0.28. Either scope it to those versions or remove it. - **"first of the two steps … prerequisite" framing vs #101** — under 0.28 the two PRs are independent (the free-threaded wheel in #101 runs GIL-off with or without this change), so "step 1 of 2" overstates the dependency. Soften to "companion, explicitness change." The "Does this PR introduce any user-facing change? No … no-op on GIL-enabled interpreters" part is accurate and can stay. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
