rstudio / httpuv

HTTP and WebSocket server package for R
Other
229 stars 86 forks source link

Add HEADERS variable to the Rook request environment #143

Closed alandipert closed 6 years ago

alandipert commented 6 years ago

Fixes #141

TODO:

wch commented 6 years ago

Some notes:

jeroen commented 6 years ago

Thanks this is great! Thank you!

jeroen commented 6 years ago

Make sure you test this on CentOS 6 though, some old compilers have trouble with iterators.

alandipert commented 6 years ago

@jeroen thank you for the feedback, I'm glad you like it!

Since making the PR we've become somewhat ambivalent about adding this now, since the info was already available, albeit in an inconvenient form. We (@jcheng5 and @wch and I) discussed passing for now, and instead holding out for a possible overhaul of the httpuv API that could include it.

What are you using it for? Would this be really helpful to you now, or would you be willing to wait?

Thanks in advance for sharing your perspective.

jeroen commented 6 years ago

I would like to use it for unit testing http requests in the curl package. What's the reason not to include it? It seems like a trivial non-breaking change?

alandipert commented 6 years ago

Thanks for your input @jeroen, scratch what I said, we're going to move forward with this 😄 I'll do the CentOS 6 testing in the very near future.

jeroen commented 6 years ago

Just checked in my centos 6 vm and all seems good !

alandipert commented 6 years ago

@wch when you have a chance can you please look at the NEWS and documentation additions and let me know if they're good?