Skip to content
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

feat(functions): add experimental invoke with streamed responses #346

Merged
merged 4 commits into from
Apr 22, 2024

Conversation

grdsdev
Copy link
Collaborator

@grdsdev grdsdev commented Apr 18, 2024

What kind of change does this PR introduce?

Close #252

Add experimental method to invoke function with streamed responses

Why is it sill experimental?

To implement the streamed responses, we need to set up a custom URLSession for setting its delegate property, which means that it doesn't use the customFetch that is provided to the functions package and all other sub-packages.

I prefer keeping it Experimental until we try to refactor the code so that all requests uses the same URLSession instance.

@dshukertjr
Copy link
Member

So this PR introduces a new public method _invoke() to handle stream response, correct? Would it be better to rename it to something like invokeStreamResponse, or something along that line maybe?

@grdsdev
Copy link
Collaborator Author

grdsdev commented Apr 22, 2024

The _ prefix is commonly used in Swift to signal an experimental or private method, but I'll rename it to _invokeWithStreamedResponse or something like that.

@dshukertjr
Copy link
Member

Didn't know that! Thanks for educating me! _invokeWithStreamedResponse sounds good!

@grdsdev grdsdev force-pushed the feat/functions/invoke-stream-response branch from ce8cedc to 8c9d3ce Compare April 22, 2024 11:44
@grdsdev grdsdev merged commit 2611b09 into main Apr 22, 2024
17 checks passed
@grdsdev grdsdev deleted the feat/functions/invoke-stream-response branch April 22, 2024 12:05
) async throws -> Response {
var request = Request(
path: functionName,
method: .post,
Copy link

Choose a reason for hiding this comment

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

This line is causing a major bug in the library. Upon upgrading our client version, we can no longer make GET/PUT/DELETE requests.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hi @jareddr thanks for pointing out the issue, a PR with the fix #367

I'll get it merged and released ASAP

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.

Ability to stream function responses
3 participants