Skip to content

Avoid response buffering for ISeq response body #527

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

Closed
wants to merge 0 commits into from

Conversation

kumarshantanu
Copy link

@kumarshantanu kumarshantanu commented Jun 4, 2025

This PR makes StreamableResponseBody implementation for ISeq avoid response buffering, which is useful when the ISeq elements are delayed at source. This also allows for a smooth Server-sent Events flow.

Consider the example usage below:

(defn make-seq []
  (Thread/sleep #_10 1000)
  (str "data: " (System/currentTimeMillis) "\n\n"))

(defn handler [request]
  {:status 200
   :headers {"Content-Type" "text/event-stream"}
   :body (repeatedly make-seq)})

@weavejester
Copy link
Member

Thanks for the PR. Can you ensure the commit messages follow the seven rules of a great git commit message as mentioned in the contributing guidelines.

Secondly, can you think of a way of testing the changes you've made?

@kumarshantanu
Copy link
Author

kumarshantanu commented Jun 4, 2025

@weavejester Thanks for the feedback - I have made the changes (edited the commit message, and reverted a commit that broke a test).

@weavejester
Copy link
Member

Thanks! Can you remove the commit you reverted and its reversion, so there's just one commit left. Can you also add a test for the change you made.

@kumarshantanu
Copy link
Author

Created PR #528 with removed unwanted commits, and a test for flushing of body.

@weavejester
Copy link
Member

For future reference, you don't need to create a new PR to do this. You can just force-push your changes and the PR will be updated. This is preferred, as it keeps the comments in one place.

@kumarshantanu
Copy link
Author

Noted. Force push after a git reset (to get rid of the unwanted commits) automatically closed the PR, so it was accidental. I should have made the new commits before the force push.

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