Skip to content

Enable dyn dispatch by dyn Config objects #383

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

Merged
merged 12 commits into from
Jun 14, 2025
Merged

Conversation

ifsheldon
Copy link
Contributor

This should supersede #125, solving #106. Different from #125, this is not a breaking change.

@64bit
Copy link
Owner

64bit commented Jun 7, 2025

This is nice!

Can you please add short/concise (one or two sentences) documentation for it to be discoverable - in README and lib.rs (for docs.rs) after byot documentation?

@ifsheldon
Copy link
Contributor Author

OK, done

@64bit
Copy link
Owner

64bit commented Jun 8, 2025

One of things that this feature must work for is dynamic dispatch same API call to multiple providers concurrently (via tokio::spawn ) - requiring Send + Sync on Config

I have suggested the corresponding test - please let me know if that looks good or if there are any concerns now that Rc can't work it?

ifsheldon and others added 7 commits June 9, 2025 22:48
Co-authored-by: Himanshu Neema <[email protected]>
Co-authored-by: Himanshu Neema <[email protected]>
Co-authored-by: Himanshu Neema <[email protected]>
Co-authored-by: Himanshu Neema <[email protected]>
Co-authored-by: Himanshu Neema <[email protected]>
Co-authored-by: Himanshu Neema <[email protected]>
Co-authored-by: Himanshu Neema <[email protected]>
@ifsheldon
Copy link
Contributor Author

Looks good to me. Thanks! I think Rc is not an issue here as long as users have Arc as an alternative.

Copy link
Owner

@64bit 64bit left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@64bit 64bit merged commit 9b3ecda into 64bit:main Jun 14, 2025
ifsheldon added a commit to ifsheldon/async-openai-wasm that referenced this pull request Jun 29, 2025
* enable dynamic dispatch

* update README with dyn dispatch example

* add doc for dyn dispatch

* Update test

Co-authored-by: Himanshu Neema <[email protected]>

* Update Config bound

Co-authored-by: Himanshu Neema <[email protected]>

* remove Rc impl

Co-authored-by: Himanshu Neema <[email protected]>

* Fix typo

Co-authored-by: Himanshu Neema <[email protected]>

* Fix typo

Co-authored-by: Himanshu Neema <[email protected]>

* Update doc

Co-authored-by: Himanshu Neema <[email protected]>

* Update README

Co-authored-by: Himanshu Neema <[email protected]>

---------

Co-authored-by: Himanshu Neema <[email protected]>

(cherry picked from commit 9b3ecda)
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.

2 participants