dora-rs / dora

DORA (Dataflow-Oriented Robotic Application) is middleware designed to streamline and simplify the creation of AI-based robotic applications. It offers low latency, composable, and distributed dataflow capabilities. Applications are modeled as directed graphs, also referred to as pipelines.

https://dora-rs.ai

Apache License 2.0

1.36k stars 69 forks source link

Update Pyo3 bounds #472

Closed Michael-J-Ward closed 1 month ago

Michael-J-Ward commented 2 months ago

pyo3 0.21 is in arrow-rs master (see https://github.com/apache/arrow-rs/pull/5566), but not yet released.

Fixes: https://github.com/dora-rs/dora/issues/437

Notes

After updating the deps, this was completely a compiler / clippy driven refactor where clippy highlights deprecation warnings, and I would update using the migration guide.

I did not go looking for any other opportunities to use the new Bounds api.

Michael-J-Ward commented 2 months ago

@haixuanTao the last deprecation warning was here.

https://github.com/dora-rs/dora/blob/bbabdc389ebe2bb638c10705c82eab307aa63e55/binaries/runtime/src/operator/python.rs#L187-L197

I think that the new bounds API makes this unnecessary, and the deprecation warning states "code not using the GIL Refs API can safely remove the use of Py::new_pool".

I included that change as the last commit, so if you find that it is still necessary it'll be an easy rollback.

haixuanTao commented 2 months ago

@haixuanTao the last deprecation warning was here.

https://github.com/dora-rs/dora/blob/bbabdc389ebe2bb638c10705c82eab307aa63e55/binaries/runtime/src/operator/python.rs#L187-L197

I think that the new bounds API makes this unnecessary, and the deprecation warning states "code not using the GIL Refs API can safely remove the use of Py::new_pool".

I included that change as the last commit, so if you find that it is still necessary it'll be an easy rollback.

That makes sense, thanks!

haixuanTao commented 2 months ago

This fix an issue that I had about stopping a dataflow that has been present for a bit!

Thanks a lot!

Can't wait to merge it.

Michael-J-Ward commented 2 months ago

@haixuanTao - It could be a few months before arrow-rs gets released.

If you don't mind pointing to arrow-rs's master, then I'll rebase this so you can merge now, and I'll update the deps once it does get released.

haixuanTao commented 2 months ago

If we merge it, we will not be able to publish on cargo, which will makes our release stuck. So I wouldn´t do it.

What we could do, is to only merge arrow version within dora-node-api-python that is distributed using pip which does not check for git packages and distribute dora-node-api-python with the latest arrow.

But not sure if we can handle multi arrow versions.

Michael-J-Ward commented 1 month ago

@haixuanTao This is rebased and ready for review.