search

openedx / XBlock

Framework for building custom learning components that run in the Open edX LMS!
https://docs.openedx.org/projects/xblock/en/latest/xblock-tutorial/index.html
Apache License 2.0
452 stars 217 forks source link

feat: Update request.py in order to allow Streaming responses #737

Closed TheoBessel closed 4 months ago

TheoBessel commented 5 months ago

(Example : needed to build correct ChatGPT XBlocks)

openedx-webhooks commented 5 months ago

Thanks for the pull request, @TheoBessel! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

mphilbrick211 commented 5 months ago

Hi @TheoBessel! Thanks for this contribution! Please let me know if you have any questions regarding submitting a CLA form.

mphilbrick211 commented 5 months ago

Hi @TheoBessel! Thanks for this contribution! Please let me know if you have any questions regarding submitting a CLA form.

Hi @TheoBessel! Just following up on this. Please let us know when you submit your CLA form. Thanks!

TheoBessel commented 5 months ago

Hi @TheoBessel! Thanks for this contribution! Please let me know if you have any questions regarding submitting a CLA form.

Hi @TheoBessel! Just following up on this. Please let us know when you submit your CLA form. Thanks!

Hi, @mphilbrick211 thanks for your reply ! I have just submitted my CLA form !

mphilbrick211 commented 5 months ago

Hi @TheoBessel! Thanks for this contribution! Please let me know if you have any questions regarding submitting a CLA form.

Hi @TheoBessel! Just following up on this. Please let us know when you submit your CLA form. Thanks!

Hi, @mphilbrick211 thanks for your reply ! I have just submitted my CLA form !

Great! The change should be reflected within 24 hours. When you re-run your checks after that, the check next to the CLA requirement should turn green.

TheoBessel commented 5 months ago

@mphilbrick211 Do I have to do anything else now ?

mphilbrick211 commented 5 months ago

Hi @TheoBessel - I will look into getting the checks re-enabled for you!

TheoBessel commented 5 months ago

Hi @TheoBessel - I will look into getting the checks re-enabled for you!

@mphilbrick211 Thanks a lot for your help !

TheoBessel commented 5 months ago

@mphilbrick211 Hello ! Any news ?

mphilbrick211 commented 5 months ago

@mphilbrick211 Hello ! Any news ?

Hi @TheoBessel - thanks for checking in, and for your patience. The checks should be enabled today. Apologies for the delay.

TheoBessel commented 5 months ago

@mphilbrick211 @nedbat Hello ! I've edited my code because it didn't pass the tests. Should I also to edit my commit messages or they are okay ? Let me know. I'll also make a PR on edx-platform and xblock-sdk in order to make the change I've proposed usable on these sides. Have a nice day !

TheoBessel commented 5 months ago

@mphilbrick211 Hello ! Have you seen my last comment ?

e0d commented 5 months ago

@TheoBessel I notice there are some commit-lint failures. Please note that we use conventional commits across Open edX projects. You can read about the details here. Can you please amend your commit messages to follow our standard?

TheoBessel commented 5 months ago

@e0d I guess it is good now ?

TheoBessel commented 5 months ago

@e0d Updated the too long commit message and added some unit tests, it should pass all workflows now (I hope)

TheoBessel commented 5 months ago

@e0d I hope this time it will be good (I removed a test I wrote and that made no sense for Streaming Responses and that was failing because of that)

TheoBessel commented 4 months ago

@e0d ?

TheoBessel commented 4 months ago

Can somebody here merge my pull request ? It passes all the checks now.

mphilbrick211 commented 4 months ago

@TheoBessel - thanks for flagging! I have requested review.

ormsbee commented 4 months ago

@TheoBessel: Please resolve conflicts and squash your commits and I'll merge.

TheoBessel commented 4 months ago

@TheoBessel: Please resolve conflicts and squash your commits and I'll merge.

I'm sorry, but I don't really know what you're talking about. I don't see any conflicts ans my commits were already amended in order to have correct messages. 🤔

ormsbee commented 4 months ago

Ah, sorry, I saw this:

Screenshot 2024-05-15 at 12 24 53 PM

And it didn't click to me that you did a merge instead of a rebase off of master (it's fine, it's just not what I'm used to). I'll squash and merge now.

openedx-webhooks commented 4 months ago

@TheoBessel 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

ormsbee commented 4 months ago

Note for anyone looking at this later: I wasn't paying attention to the top line of the squashed commit message, and merged it in with the feat!: it got from the original title of this PR, rather than feat:, which is what it should be. This PR did not take away anything or break backwards compatibility.

I'll make a new PR to bump the version of this package and tag it for PyPI.