Separate into internal object-store crate

roeap / object-store-python

Python bindings and arrow integration for the rust object_store crate.

Apache License 2.0

51 stars 8 forks source link

Separate into internal object-store crate #4

Closed kylebarron closed 7 months ago

kylebarron commented 7 months ago

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 https://github.com/roeap/object-store-python/issues/3

roeap commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

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 commented 7 months ago

In https://github.com/roeap/object-store-python/pull/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 https://github.com/PyO3/pyo3/pull/3540 and https://github.com/PyO3/pyo3/issues/1632

roeap commented 7 months ago

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

roeap commented 7 months ago

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