You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "ntjohnson1 (via GitHub)" <gi...@apache.org> on 2025/09/22 15:08:24 UTC
[PR] Allow Python Threading Around Session Context [datafusion-python]
ntjohnson1 opened a new pull request, #1248:
URL: https://github.com/apache/datafusion-python/pull/1248
# Which issue does this PR close?
Happy to write a more focussed issue since this is just specific to SessionContext.
Closes #366
# Rationale for this change
The issues provides a minimal repro for my motivating use case.
# What changes are included in this PR?
Makes `SessionContext` frozen since all the methods are fairly thin wrappers around the rust struct which manages mutability already.
Since all the existing tests passed (at least locally) that should be reasonable confidence this isn't totally flawed. I could add a test related to my repro just to show threads work now.
# Are there any user-facing changes?
There shouldn't be.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Allow Python Threading Around Session Context [datafusion-python]
Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on PR #1248:
URL: https://github.com/apache/datafusion-python/pull/1248#issuecomment-3322028973
I think a better PR description might be "Make pyclasses `frozen` to use our own interior mutability"
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make Session Context `pyclass` frozen so interior mutability is only managed by rust [datafusion-python]
Posted by "kylebarron (via GitHub)" <gi...@apache.org>.
kylebarron commented on PR #1248:
URL: https://github.com/apache/datafusion-python/pull/1248#issuecomment-3324279334
Looks great! I think in terms of title I was just confused because it doesn't _directly_ relate to threading at all; it's just an indirect incompatibility through the pyo3 implementation
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make Session Context `pyclass` frozen so interior mutability is only managed by rust [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer merged PR #1248:
URL: https://github.com/apache/datafusion-python/pull/1248
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make Session Context `pyclass` frozen so interior mutability is only managed by rust [datafusion-python]
Posted by "ntjohnson1 (via GitHub)" <gi...@apache.org>.
ntjohnson1 commented on PR #1248:
URL: https://github.com/apache/datafusion-python/pull/1248#issuecomment-3324296817
> I'd say all pyclasses should be frozen by default (and I think pyo3 intends to make that the default in the future?).
I can create an issue for follow up on this to move more classes in the repo to frozen. Don't think I'll have a chance to get to it immediately. Mostly just solved this because by the time I root caused the issue it was basically fixed.
Re: pyo3 frozen default that is my understanding as well based on the [note under the code block](https://pyo3.rs/main/class.html#frozen-classes-opting-out-of-interior-mutability)
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make Session Context `pyclass` frozen so interior mutability is only managed by rust [datafusion-python]
Posted by "ntjohnson1 (via GitHub)" <gi...@apache.org>.
ntjohnson1 commented on PR #1248:
URL: https://github.com/apache/datafusion-python/pull/1248#issuecomment-3323400431
> It doesn't look like there are any actual changes here? You're just switching to not use `&mut`?
Correct. Basically marked the class as frozen to make sure no `mut self` accidentally creep in. When not `mut self` then python threads are enabled since we aren't trying to borrow mutable versions of the python wrapper twice. Then I believe the rust compiler verifies nothing deeper in the accesses actual needed a mutable version of the rust Session Context and are safe for concurrency (things are `Arc` etc all the way down).
> I think a better PR description might be "Make pyclasses frozen to use our own interior mutability"
Updated the description. Can update to your suggestion verbatim if still unclear.
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] Make Session Context `pyclass` frozen so interior mutability is only managed by rust [datafusion-python]
Posted by "timsaucer (via GitHub)" <gi...@apache.org>.
timsaucer commented on PR #1248:
URL: https://github.com/apache/datafusion-python/pull/1248#issuecomment-3328250545
Thank you @ntjohnson1 and @kylebarron
--
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: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org