-
Notifications
You must be signed in to change notification settings - Fork 240
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
add basic wasm support #122
Conversation
# Conflicts: # async-openai/Cargo.toml # async-openai/src/client.rs
and remove some unnecessary feature gates
Hi @ifsheldon Thank you for all of your good work! This needs some work on documentation:
I also have few questions:
Regarding your note about testing on OpenAI in the other PR - I'll be happy to help test changes and let you know how it goes - please expect some delay from my side though. |
For documentation, I will add more when I have some time.
Sure, but I don't know much about the feature flags that existing in main now, tls related ones.
I think it will work nonetheless. Haven't tries these though. As far as I know
Basically anything related to files is not supported, except finetuning since it only needs strings in the code, not directly related to files. I will write more in the documentation, though. Future work can be removing |
@64bit I've added documentation and an example. Can you review these? Thanks! |
Thank you for updates, please expect some delays as previously mentioned, to provide you context - some older PRs & spec update issues needs attention and given that this is a big new feature and requires testing - it might be a while before I get to this. In the meantime would you mind closing your other PRs which are no longer relevant? Thank you for your patience. |
@64bit do you have any plan to merge this? or any comments? I saw new features got added recently, which makes this PR more intertwined and need more testing. If you have no intention to merge this, I can close this. Or I can try to follow up new features and see if they complies on wasm. |
Hi @ifsheldon , The primary & bare minimum purpose of this crate is to work with OpenAI API - if it doesn't work with API this crate should not exist - that's why some PRs were merged before any other open PRs. That said, I'm sure community would love to have wasm support and I do too - however I'm just limited by my bandwidth. This PR is a big feature and requires testing and doubles the surface area for testing (wasm and non-wasm) - without breaking existing features - all that creates extra work for me for every single update now and in the future. As much as I want to merge your contributions, above are the practical reasons that I cannot accept this PR anytime soon, I'm not sure when I could spend time on this, and so I'm really sorry about that. It would nice to have your work in this PR published - so here are few options:
Please let me know what you think and any alternative path forward I'm open to hear them. Thank you |
OK. Either way I need to maintain the code, so I'd rather not to distract developers by forking a new crate. So I think 1. is good for me. I will try to keep up with OpenAI new features soon. When it's done, you can just release an alpha. |
Thank you for your willingness! Let's ship it, I'm thinking to create 'experiments' branch to merge into and other related future changes |
# Conflicts: # async-openai/Cargo.toml # async-openai/src/client.rs # async-openai/src/lib.rs # async-openai/src/types/impls.rs # async-openai/src/types/types.rs
@64bit Great! I've made few changes to get features in this branch in sync with main. My example has been updated to support OpenAI APIs, and I've tested it with Azure OpenAI and OpenAI. The next step may be to separate file paths from file binaries in Input structs. Like |
This is based on #120 and #121.
To summarize:
enable_tokio
andenable_backoff
backoff
is somehow difficult to make it support wasm, so I just decided to gate it under a featureOpenAIEventStream
but it should not matter as long as users only use it inwhile let await
loops since it also implementsStream
reqwest-eventsource
This should close #102 unless file-related ops are wanted.
I tested the code by
cargo build --target wasm32-unknown-unknown --no-default-features --features wasm
and an web app.The code is
Cargo.toml