feat(bindings/python): add check in py by asukaminato0721 · Pull Request #5973 · apache/opendal

asukaminato0721 · 2025-04-07T05:04:46Z

Which issue does this PR close?

part of #5963.

sub issue

sync check in opendal need work around #5995

Rationale for this change

What changes are included in this PR?

user can use blocking check and unblocking check in py binding

Are there any user-facing changes?

asukaminato0721 · 2025-04-07T20:38:02Z

    @pytest.mark.need_capability("list")
    def test_sync_copy(service_name, operator, async_operator):
>       operator.check()
E       pyo3_runtime.PanicException: this functionality requires a Tokio context

only this failed.

yihong0618 · 2025-04-08T00:27:27Z

    @pytest.mark.need_capability("list")
    def test_sync_copy(service_name, operator, async_operator):
>       operator.check()
E       pyo3_runtime.PanicException: this functionality requires a Tokio context

only this failed.

interesting will take a look and try to learn it today

yihong0618 · 2025-04-08T01:43:19Z

reproduce in local

bt

thread '<unnamed>' panicked at /Users/hyi/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sqlx-core-0.8.3/src/pool/connection.rs:208:13:
this functionality requires a Tokio context
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: sqlx_core::rt::missing_rt
   3: sqlx_core::rt::spawn
   4: <sqlx_core::pool::connection::PoolConnection<DB> as core::ops::drop::Drop>::drop
   5: core::ptr::drop_in_place<sqlx_core::pool::connection::PoolConnection<sqlx_sqlite::database::Sqlite>>
   6: core::ptr::drop_in_place<sqlx_core::pool::executor::<impl sqlx_core::executor::Executor for &sqlx_core::pool::Pool<sqlx_sqlite::database::Sqlite>>::fetch_many<sqlx_core::query::Query<sqlx_sqlite::database::Sqlite,sqlx_sqlite::arguments::SqliteArguments>>::{{closure}}::{{closure}}>
   7: core::ptr::drop_in_place<sqlx_core::ext::async_stream::TryAsyncStream<either::Either<sqlx_sqlite::query_result::SqliteQueryResult,sqlx_sqlite::row::SqliteRow>>>
   8: core::ptr::drop_in_place<futures_util::stream::stream::map::Map<core::pin::Pin<alloc::boxed::Box<dyn futures_core::stream::Stream+Item = core::result::Result<alloc::string::String,sqlx_core::error::Error>+core::marker::Send>>,<opendal::services::sqlite::backend::Adapter as opendal::raw::adapters::kv::api::Adapter>::scan::{{closure}}::{{closure}}::{{closure}}>>
   9: core::ptr::drop_in_place<futures_util::stream::try_stream::try_filter_map::TryFilterMap<core::pin::Pin<alloc::boxed::Box<dyn futures_core::stream::Stream+Item = core::result::Result<either::Either<sqlx_sqlite::query_result::SqliteQueryResult,(alloc::string::String,)>,sqlx_core::error::Error>+core::marker::Send>>,sqlx_core::query_as::QueryAs<sqlx_sqlite::database::Sqlite,(alloc::string::String,),sqlx_sqlite::arguments::SqliteArguments>::fetch<&sqlx_core::pool::Pool<sqlx_sqlite::database::Sqlite>>::{{closure}}::{{closure}},sqlx_core::query_as::QueryAs<sqlx_sqlite::database::Sqlite,(alloc::string::String,),sqlx_sqlite::arguments::SqliteArguments>::fetch<&sqlx_core::pool::Pool<sqlx_sqlite::database::Sqlite>>::{{closure}}>>
  10: core::ptr::drop_in_place<futures_util::stream::stream::map::Map<core::pin::Pin<alloc::boxed::Box<dyn futures_core::stream::Stream+Item = core::result::Result<alloc::string::String,sqlx_core::error::Error>+core::marker::Send>>,<opendal::services::sqlite::backend::Adapter as opendal::raw::adapters::kv::api::Adapter>::scan::{{closure}}::{{closure}}::{{closure}}>>
  11: core::ptr::drop_in_place<futures_util::stream::stream::map::Map<core::pin::Pin<alloc::boxed::Box<dyn futures_core::stream::Stream+Item = core::result::Result<alloc::string::String,sqlx_core::error::Error>+core::marker::Send>>,<opendal::services::sqlite::backend::Adapter as opendal::raw::adapters::kv::api::Adapter>::scan::{{closure}}::{{closure}}::{{closure}}>>
  12: <opendal::services::sqlite::backend::ouroboros_impl_sqlite_scanner::SqliteScanner as core::ops::drop::Drop>::drop
  13: core::ptr::drop_in_place<opendal::raw::oio::list::hierarchy_list::HierarchyLister<opendal::raw::adapters::kv::backend::KvLister<opendal::services::sqlite::backend::ouroboros_impl_sqlite_scanner::SqliteScanner>>>
  14: core::ptr::drop_in_place<opendal::layers::blocking::BlockingWrapper<alloc::boxed::Box<dyn opendal::raw::oio::list::api::ListDyn>>>
  15: core::ptr::drop_in_place<opendal::layers::retry::RetryWrapper<alloc::boxed::Box<dyn opendal::raw::oio::list::api::BlockingList>,opendal::layers::retry::DefaultRetryInterceptor>>
  16: core::ptr::drop_in_place<opendal::layers::concurrent_limit::ConcurrentLimitWrapper<alloc::boxed::Box<dyn opendal::raw::oio::list::api::BlockingList>>>
  17: opendal::types::operator::blocking_operator::BlockingOperator::check
  18: opendal_python::operator::Operator::__pymethod_check__
  19: pyo3::impl_::trampoline::trampoline
  20: opendal_python::operator::<impl pyo3::impl_::pyclass::PyMethods<opendal_python::operator::Operator> for pyo3::impl_::pyclass::PyClassImplCollector<opendal_python::operator::Operator>>::py_methods::ITEMS::trampoline
  21: _method_vectorcall_NOARGS.llvm.11409851845177106966
  22: __PyEval_EvalFrameDefault
  23: __PyObject_Call_Prepend
  24: _slot_tp_call
  25: __PyEval_EvalFrameDefault
  26: __PyObject_Call_Prepend
  27: _slot_tp_call
  28: __PyEval_EvalFrameDefault
  29: __PyObject_Call_Prepend
  30: _slot_tp_call
  31: __PyEval_EvalFrameDefault
  32: __PyObject_Call_Prepend
  33: _slot_tp_call
  34: __PyEval_EvalFrameDefault
  35: __PyObject_Call_Prepend
  36: _slot_tp_call
  37: __PyEval_EvalFrameDefault
  38: _PyEval_EvalCode
  39: _run_eval_code_obj
  40: _run_mod.llvm.1117032873449786082
  41: _pyrun_file
  42: __PyRun_SimpleFileObject
  43: __PyRun_AnyFileObject
  44: _pymain_run_file_obj
  45: _pymain_run_file
  46: _Py_RunMain
  47: _pymain_main
  48: _Py_BytesMain
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

yihong0618 · 2025-04-08T01:51:24Z

@asukaminato0721

you can change to this and test

and we may create an issue about this

    pub fn check(&self) -> PyResult<()> {
        let runtime = pyo3_async_runtimes::tokio::get_runtime();
        let _guard = runtime.enter();
        
        self.core.check().map_err(format_pyerr)
    }

asukaminato0721 · 2025-04-08T04:32:47Z

now only Docs / build-website (pull_request) failed. All tests are passed.

yihong0618 · 2025-04-08T04:34:28Z

bindings/python/src/operator.rs

+        let runtime = pyo3_async_runtimes::tokio::get_runtime();
+        let _guard = runtime.enter();


this is a temp solution maybe we can find a better solution
since we have this in build operator side

It's wired that only this function need workaround. If check failed, the list should also fail too. I'm guessing all services related to sqlx will fail as well.

We should identify the root cause and fix it in the core instead. Having such a workaround in the bindings doesn't seem ideal to me.

It's wired that only this function need workaround. If check failed, the list should also fail too. I'm guessing all services related to sqlx will fail as well.

We should identify the root cause and fix it in the core instead. Having such a workaround in the bindings doesn't seem ideal to me.

yes agree and will try to help these days

add a bit test, seems that list doesn't fail

services related to sqlx will fail as well.
seems yes postgres and mysql are skipped

yihong0618 · 2025-04-08T04:35:18Z

others LGTM, will try to find why the doc failed

Xuanwo

-1 to put workaround in bindings.

yihong0618 · 2025-04-08T09:18:20Z

did not reproduce why the doc failed...

Kilerd · 2025-04-08T14:13:37Z

reproduce in local

bt

thread '<unnamed>' panicked at /Users/hyi/.cargo/registry/src/index.crates.io-6f17d22bba15001f/sqlx-core-0.8.3/src/pool/connection.rs:208:13:
this functionality requires a Tokio context
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: sqlx_core::rt::missing_rt
   3: sqlx_core::rt::spawn
   4: <sqlx_core::pool::connection::PoolConnection<DB> as core::ops::drop::Drop>::drop
   5: core::ptr::drop_in_place<sqlx_core::pool::connection::PoolConnection<sqlx_sqlite::database::Sqlite>>
   6: core::ptr::drop_in_place<sqlx_core::pool::executor::<impl sqlx_core::executor::Executor for &sqlx_core::pool::Pool<sqlx_sqlite::database::Sqlite>>::fetch_many<sqlx_core::query::Query<sqlx_sqlite::database::Sqlite,sqlx_sqlite::arguments::SqliteArguments>>::{{closure}}::{{closure}}>
   7: core::ptr::drop_in_place<sqlx_core::ext::async_stream::TryAsyncStream<either::Either<sqlx_sqlite::query_result::SqliteQueryResult,sqlx_sqlite::row::SqliteRow>>>
   8: core::ptr::drop_in_place<futures_util::stream::stream::map::Map<core::pin::Pin<alloc::boxed::Box<dyn futures_core::stream::Stream+Item = core::result::Result<alloc::string::String,sqlx_core::error::Error>+core::marker::Send>>,<opendal::services::sqlite::backend::Adapter as opendal::raw::adapters::kv::api::Adapter>::scan::{{closure}}::{{closure}}::{{closure}}>>
   9: core::ptr::drop_in_place<futures_util::stream::try_stream::try_filter_map::TryFilterMap<core::pin::Pin<alloc::boxed::Box<dyn futures_core::stream::Stream+Item = core::result::Result<either::Either<sqlx_sqlite::query_result::SqliteQueryResult,(alloc::string::String,)>,sqlx_core::error::Error>+core::marker::Send>>,sqlx_core::query_as::QueryAs<sqlx_sqlite::database::Sqlite,(alloc::string::String,),sqlx_sqlite::arguments::SqliteArguments>::fetch<&sqlx_core::pool::Pool<sqlx_sqlite::database::Sqlite>>::{{closure}}::{{closure}},sqlx_core::query_as::QueryAs<sqlx_sqlite::database::Sqlite,(alloc::string::String,),sqlx_sqlite::arguments::SqliteArguments>::fetch<&sqlx_core::pool::Pool<sqlx_sqlite::database::Sqlite>>::{{closure}}>>
  10: core::ptr::drop_in_place<futures_util::stream::stream::map::Map<core::pin::Pin<alloc::boxed::Box<dyn futures_core::stream::Stream+Item = core::result::Result<alloc::string::String,sqlx_core::error::Error>+core::marker::Send>>,<opendal::services::sqlite::backend::Adapter as opendal::raw::adapters::kv::api::Adapter>::scan::{{closure}}::{{closure}}::{{closure}}>>
  11: core::ptr::drop_in_place<futures_util::stream::stream::map::Map<core::pin::Pin<alloc::boxed::Box<dyn futures_core::stream::Stream+Item = core::result::Result<alloc::string::String,sqlx_core::error::Error>+core::marker::Send>>,<opendal::services::sqlite::backend::Adapter as opendal::raw::adapters::kv::api::Adapter>::scan::{{closure}}::{{closure}}::{{closure}}>>
  12: <opendal::services::sqlite::backend::ouroboros_impl_sqlite_scanner::SqliteScanner as core::ops::drop::Drop>::drop
  13: core::ptr::drop_in_place<opendal::raw::oio::list::hierarchy_list::HierarchyLister<opendal::raw::adapters::kv::backend::KvLister<opendal::services::sqlite::backend::ouroboros_impl_sqlite_scanner::SqliteScanner>>>
  14: core::ptr::drop_in_place<opendal::layers::blocking::BlockingWrapper<alloc::boxed::Box<dyn opendal::raw::oio::list::api::ListDyn>>>
  15: core::ptr::drop_in_place<opendal::layers::retry::RetryWrapper<alloc::boxed::Box<dyn opendal::raw::oio::list::api::BlockingList>,opendal::layers::retry::DefaultRetryInterceptor>>
  16: core::ptr::drop_in_place<opendal::layers::concurrent_limit::ConcurrentLimitWrapper<alloc::boxed::Box<dyn opendal::raw::oio::list::api::BlockingList>>>
  17: opendal::types::operator::blocking_operator::BlockingOperator::check
  18: opendal_python::operator::Operator::__pymethod_check__
  19: pyo3::impl_::trampoline::trampoline
  20: opendal_python::operator::<impl pyo3::impl_::pyclass::PyMethods<opendal_python::operator::Operator> for pyo3::impl_::pyclass::PyClassImplCollector<opendal_python::operator::Operator>>::py_methods::ITEMS::trampoline
  21: _method_vectorcall_NOARGS.llvm.11409851845177106966
  22: __PyEval_EvalFrameDefault
  23: __PyObject_Call_Prepend
  24: _slot_tp_call
  25: __PyEval_EvalFrameDefault
  26: __PyObject_Call_Prepend
  27: _slot_tp_call
  28: __PyEval_EvalFrameDefault
  29: __PyObject_Call_Prepend
  30: _slot_tp_call
  31: __PyEval_EvalFrameDefault
  32: __PyObject_Call_Prepend
  33: _slot_tp_call
  34: __PyEval_EvalFrameDefault
  35: __PyObject_Call_Prepend
  36: _slot_tp_call
  37: __PyEval_EvalFrameDefault
  38: _PyEval_EvalCode
  39: _run_eval_code_obj
  40: _run_mod.llvm.1117032873449786082
  41: _pyrun_file
  42: __PyRun_SimpleFileObject
  43: __PyRun_AnyFileObject
  44: _pymain_run_file_obj
  45: _pymain_run_file
  46: _Py_RunMain
  47: _pymain_main
  48: _Py_BytesMain
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

the reason why the error raise is, services depended on sqlx, like sqlite, mysql and postgres, need to drop connections when exiting. and due the the limitation of rust Drop trait, they need to spawn an async task to close connection on Drop trait, in other words, it's the async drop scenario.

after going through the implement of sqlx, it spawn the task of closing conn via crate::rt::spawn (https://github.com/launchbadge/sqlx/blob/082aed5c2b6e68172bf29c377c3f5c87ca17cde4/sqlx-core/src/pool/connection.rs#L208). while there is no tokio runtime in python environment.

P.S. it might crash on those services which depended on sqlx: sqlite, mysql, postgres

Xuanwo · 2025-04-09T05:01:41Z

I believe this issue is complex enough that we need a separate one for it.

yihong0618 · 2025-04-17T01:55:56Z

let's merge main to see the error still exists?

bindings/python/src/operator.rs

Xuanwo

Thank you @asukaminato0721 for working on this!

asukaminato0721 requested review from Zheaoli and messense as code owners April 7, 2025 05:04

dosubot bot added the size:S This PR changes 10-29 lines, ignoring generated files. label Apr 7, 2025

asukaminato0721 force-pushed the blocking-check branch from 5a78c41 to eb1ae0f Compare April 7, 2025 05:05

dosubot bot added bindings/python releases-note/feat The PR implements a new feature or has a title that begins with "feat" labels Apr 7, 2025

asukaminato0721 force-pushed the blocking-check branch from 6915dd0 to 7173e00 Compare April 7, 2025 19:23

dosubot bot added size:M This PR changes 30-99 lines, ignoring generated files. and removed size:S This PR changes 10-29 lines, ignoring generated files. labels Apr 7, 2025

asukaminato0721 changed the title ~~add check in py~~ feat(bindings/python): add check in py Apr 7, 2025

asukaminato0721 requested a review from Xuanwo as a code owner April 7, 2025 20:25

asukaminato0721 added 9 commits April 8, 2025 13:19

add check in py

6a7b33e

fix signature

fefb382

fmt

1d08f30

add test

f0a56a4

async_operator

839e564

test file

e1d47e7

add check cap

9514b98

use list cap

87cce94

try fix

02e3c18

asukaminato0721 force-pushed the blocking-check branch from 7239eec to 02e3c18 Compare April 8, 2025 04:19

recover fmt

9de8f42

yihong0618 reviewed Apr 8, 2025

View reviewed changes

Xuanwo requested changes Apr 8, 2025

View reviewed changes

asukaminato0721 added 3 commits April 8, 2025 19:33

try test list

e4d4c92

Update test_sync_check.py

856e861

first do list

9e17ed9

asukaminato0721 mentioned this pull request Apr 9, 2025

sync check in opendal need work around #5995

Closed

asukaminato0721 commented Apr 17, 2025

View reviewed changes

bindings/python/src/operator.rs Outdated Show resolved Hide resolved

asukaminato0721 added 3 commits April 17, 2025 14:54

rm workaround

86eb85a

Merge branch 'main' into blocking-check

64e0cf0

Update operator.rs

23be8f1

Xuanwo approved these changes Apr 17, 2025

View reviewed changes

dosubot bot added the lgtm This PR has been approved by a maintainer label Apr 17, 2025

Xuanwo merged commit f97c0fb into apache:main Apr 17, 2025
62 of 63 checks passed

asukaminato0721 deleted the blocking-check branch April 17, 2025 07:51

AsperforMias approved these changes Apr 17, 2025

View reviewed changes

		let runtime = pyo3_async_runtimes::tokio::get_runtime();
		let _guard = runtime.enter();

Comments

Conversation

asukaminato0721 commented Apr 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Uh oh!

asukaminato0721 commented Apr 7, 2025

Uh oh!

yihong0618 commented Apr 8, 2025

Uh oh!

yihong0618 commented Apr 8, 2025

Uh oh!

yihong0618 commented Apr 8, 2025

Uh oh!

asukaminato0721 commented Apr 8, 2025

Uh oh!

yihong0618 Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Xuanwo Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

yihong0618 Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

asukaminato0721 Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

yihong0618 Apr 8, 2025

Choose a reason for hiding this comment

Uh oh!

yihong0618 commented Apr 8, 2025

Uh oh!

Xuanwo left a comment

Choose a reason for hiding this comment

Uh oh!

yihong0618 commented Apr 8, 2025

Uh oh!

Kilerd commented Apr 8, 2025

Uh oh!

Xuanwo commented Apr 9, 2025

Uh oh!

yihong0618 commented Apr 17, 2025

Uh oh!

Uh oh!

Xuanwo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

asukaminato0721 commented Apr 7, 2025 •

edited

Loading

yihong0618 Apr 8, 2025 •

edited

Loading