chore(toolbox-core): Create helper sync tool and sync client protected methods to load tools and return future

[anubhav756]

This change adds protected helper methods to return a future while loading tool/toolset.

These helpers are intended for use in an upcoming PR (#229) where we wrap toolbox-langchain onto toolbox-core.

[kurtisvg]

Could we get some more explanation? Why do we want to expose these are properties? Do we need to access them outside of the class? What's an example of the intended use?

My concern is we are taking internal implementation details and turning them into public APIs, which limits how we can change things in the future.

[anubhav756]

Could we get some more explanation? Why do we want to expose these are properties? Do we need to access them outside of the class? What's an example of the intended use?

My concern is we are taking internal implementation details and turning them into public APIs, which limits how we can change things in the future.

These properties are intended for use in an upcoming PR (#229) where we wrap toolbox-langchain onto toolbox-core. Specifically, toolbox-langchain will need to access toolbox-core's event loop/thread via these properties to schedule tasks and ensure operations run in the correct context. (screenshot & screenshot).

Added this to the PR's description as well.

[kurtisvg]

These properties are intended for use in an upcoming PR (#229) where we wrap toolbox-langchain onto toolbox-core. Specifically, toolbox-langchain will need to access toolbox-core's event loop/thread via these properties to schedule tasks and ensure operations run in the correct context. (screenshot & screenshot).

Added this to the PR's description as well.

Thanks for the additional context. I looked at the other PR and am concerned it's not the right approach.

Can we just use asyncio.to_thread instead? In this scenario, we don't have to worry about the connection pool passing through the thread/loop boundary.

Alternatively, we could add async functions to the sync classes so they can be used for both.

[anubhav756]

These properties are intended for use in an upcoming PR (#229) where we wrap toolbox-langchain onto toolbox-core. Specifically, toolbox-langchain will need to access toolbox-core's event loop/thread via these properties to schedule tasks and ensure operations run in the correct context. (screenshot & screenshot).
Added this to the PR's description as well.

Thanks for the additional context. I looked at the other PR and am concerned it's not the right approach.

Can we just use asyncio.to_thread instead? In this scenario, we don't have to worry about the connection pool passing through the thread/loop boundary.

Alternatively, we could add async functions to the sync classes so they can be used for both.

Thanks for the suggestions. I resonate more with adding async functions to sync classes (I've made them protected funcs for now), and was able to remove the properties that expose internal state. Hope that alleviates your concerns 🙂

[anubhav756]

Per offline discussion, we are changing the approach to use asyncio.to_thread() in the wrapping toolbox-langchain async interfaces. So this PR essentially is not required at all since no helpers or additional changes are required in the toolbox-core package. The changes in the toolbox-langchain package are done in #229.