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

Allow to specify a status for streaming endpoints #966

Merged
merged 1 commit into from Jun 9, 2018

Conversation

Projects
None yet
4 participants
@jvanbruegge
Copy link
Contributor

commented May 27, 2018

Currently streaming endpoints are always returning 200 OK. With this it is not possible to implement e.g. Range Requests, as those need to return 206 Partial Content. This PR changes this.

@alpmestan

This comment has been minimized.

Copy link
Contributor

commented May 28, 2018

Your other PR has been merged, so you can rebase this one now :)

@jvanbruegge jvanbruegge force-pushed the jvanbruegge:stream-code branch from 6153866 to dbbe9b7 May 28, 2018

@jvanbruegge jvanbruegge changed the title WIP: Allow to specify a status for streaming endpoints Allow to specify a status for streaming endpoints May 28, 2018

@jvanbruegge

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2018

Rebased

@alpmestan

This comment has been minimized.

Copy link
Contributor

commented May 28, 2018

Modulo updating docs/tutorial/etc, this looks to me. @phadej @gbaz Any objection?

@phadej

This comment has been minimized.

Copy link
Member

commented May 28, 2018

Test(s) would be great as well.

@jvanbruegge

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2018

How would I write a test for this? I guess I somehow have to assert the status code, but I am not sure how to get that status with clientM. I am only using servant-server myself

@jvanbruegge

This comment has been minimized.

Copy link
Contributor Author

commented May 28, 2018

responseStatusCode, thats what I searched for. I only looked at the StreamSpec tests, my fault

@jvanbruegge

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2018

Currently streaming endpoints only return a ResultStream, there is no way to get the status code. Should I add a Status entry on the ResultStream type, in addition to the IO Continuation?
I'm not sure how you would want to handle this. I haven't had (and will not for at least this week) have time to look deeper into the servant-client code base to take a look how normal responses handle this.

@jvanbruegge

This comment has been minimized.

Copy link
Contributor Author

commented Jun 1, 2018

I can confirm that this works. I was able to implement HTTP range requests with Servant and Conduit: jvanbruegge/media-goggler@b77da5e

@phadej phadej merged commit f53370b into haskell-servant:master Jun 9, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jvanbruegge jvanbruegge deleted the jvanbruegge:stream-code branch Jun 9, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.