-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Add comprehensive streaming responses documentation to Custom HTTP API Endpoints #3192
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 comprehensive streaming responses documentation to Custom HTTP API Endpoints #3192
Conversation
Deploying wasp-docs-on-main with
|
Latest commit: |
beb5026
|
Status: | ✅ Deploy successful! |
Preview URL: | https://fc3925b9.wasp-docs-on-main.pages.dev |
Branch Preview URL: | https://copilot-fix-0b836315-72b9-4d.wasp-docs-on-main.pages.dev |
@copilot Don't worry about the versioned docs, I'll take care of it. Let's mention the use case of streaming responses from LLMs - common these days. Let's make sure to use |
Updated the streaming documentation to address your feedback! Changes in commit af4b450: ✅ Added LLM use case prominently in the introduction - now highlights streaming AI-generated content like ChatGPT The documentation now uses simpler, more direct language and showcases modern streaming use cases. The examples are consolidated using auto-js for cleaner maintenance. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot I've left you a review. Please address the comments and remove the extra file apis_test.md
589a2e1
to
8fc3098
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally ok, some suggestions:
@copilot address the comments by @cprecioso leave out the "I'd add a more relevant example here." comment, I'll deal with that one myself |
Addressed all @cprecioso's suggestions in commit dd86ed4: ✅ Import cleanup: Removed unused Left out the "more relevant example" comment as requested - that one is yours to handle. |
…I Endpoints Co-authored-by: infomiho <2223680+infomiho@users.noreply.github.com>
Co-authored-by: infomiho <2223680+infomiho@users.noreply.github.com>
…organization Co-authored-by: infomiho <2223680+infomiho@users.noreply.github.com>
Co-authored-by: infomiho <2223680+infomiho@users.noreply.github.com>
Co-authored-by: infomiho <2223680+infomiho@users.noreply.github.com>
d4a5c5c
to
b908ad8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not approving since I think you forgot to push some changes
} | ||
``` | ||
|
||
#### Using Axios |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we want to show users how to do it with fetch
and with axios
? I think we can remove the axios section tbh (being that fetch
is lighter and more modern). Or at the very least, tell the user why they'd want to use axios
instead of fetch
and vice-versa.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have an api
with auth provided that users can just import and that's what we push them to use for custom HTTP endpoints. Since Axios doesn't support streaming responses, we introduce fetch
here for the first time in our examples, ever. So I wanted to also give them an option if they want to stick with Axios. I guess the way forward would be to upgrade our internal API not to use Axios and to use something fetch
based e.g. https://github.com/suhaotian/xior
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep this is mentioned in #2935
As I said, at least I would mention why users should take one or other.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought was currently written explains that:
- downside:
Axios does not natively support streaming responses
- upside:
you don't have to provide the session token manually, as the Axios wrapper from
wasp/client/apidoes that for you.
Should I add something else here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmmm IMO that is just a description but that doesn't provide clear guidance. I think we should focus on when are this downsides or upsides important for me?
Anyway it's quite a small thing so accepting, whatever you choose is good.
} | ||
``` | ||
|
||
#### Using Axios |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmmm IMO that is just a description but that doesn't provide clear guidance. I think we should focus on when are this downsides or upsides important for me?
Anyway it's quite a small thing so accepting, whatever you choose is good.
Updated the wording to better explain the trade-offs between fetch and Axios 😄 |
Closes #3191