Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Separate into internal object-store crate #4

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

kylebarron
Copy link
Contributor

@kylebarron kylebarron commented Feb 7, 2024

Considerations:

  • Most dependencies and rust code are moved to the internal object-store-internal crate. Only the actual #[pymodule] is in the existing python crate.
  • The internal rlib crate would be published to crates.io, and so naming and struct visibility should be considered. I.e. I'd recommend object-store-python as the name of the internal crate, as that would be descriptive for people to see on crates.io.
  • I also haven't given thought yet/tried to how a third party will actually use object-store-internal. The hope is that it will be possible for a third party rust-python binding

Closes #3

@kylebarron kylebarron mentioned this pull request Feb 7, 2024
@roeap
Copy link
Owner

roeap commented Feb 8, 2024

Thanks for putting this up @kylebarron.

Before merging this, I'd just like to see if @tustvold has an opinion on this - since providing python integrations behind a feature flag is not uncommon in the arrow-ecosystem at large, maybe it may even make sense to completely upstream this to object_store behind a feature gate?

Not sure if this would even be a good idea, especially the Arrow file system stuff, but just making sure...

@kylebarron
Copy link
Contributor Author

arrow-rs implements conversions with existing Python objects (e.g. importing from and exporting to pyarrow arrays), rather than defining any new Python objects itself. Thus I'd have guessed this would be out of scope for object-store, but similarly interested in tustvold's thoughts.

@tustvold
Copy link

I think having python bindings for the object_store crate would be pretty compelling, although historically the sticking point has been the lack of a good story for async across the FFI boundary. I am not sure if this has improved, I see this crate is using block_on which is perhaps unfortunate, and so my feelings on the matter would likely depend on whether this state of play has improved recently.

@kylebarron
Copy link
Contributor Author

kylebarron commented Feb 11, 2024

In #6 I implemented async bindings to Python using pyo3-asyncio, which is the best thing available today. It looks like the next release of pyo3 will have better built-in support for async. See PyO3/pyo3#3540 and PyO3/pyo3#1632

@roeap
Copy link
Owner

roeap commented Feb 16, 2024

@tustvold @kylebarron - maybe we just start here, and see where this ends up. If we are happy then upstream it..

@roeap
Copy link
Owner

roeap commented Feb 16, 2024

@kylebarron thanks for doing this and sorry for the delay, more keeps me from doing fun stuff :D.

@roeap roeap merged commit fa6d9a7 into roeap:main Feb 16, 2024
5 checks passed
@kylebarron kylebarron deleted the kyle/object-store-python-crate branch February 16, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Expose Python interface for other Rust applications?
3 participants