-
Notifications
You must be signed in to change notification settings - Fork 505
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
http_header_envs option to include http headers as an env var #835
Conversation
Dear @danlester |
I'll take a look soon, I like the idea. Does this follow any standard, like
CGI?
(from mobile phone)
…On Sat, May 22, 2021, 14:41 nariyar ***@***.***> wrote:
Dear @danlester <https://github.com/danlester>
Is this merge expected to happen anytime soon? This would be a huge help.
Will unblock progress of our work as we needed exactly this. Thanks in
advance..
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#835 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AANPEPOOWDWRDZYHQLDKSLDTO6Q53ANCNFSM4X4IW2VQ>
.
|
@nariyar Thank you for your comment. I'm not a Voilà maintainer so can't merge it myself. Marten would have to do it (which is why he said he'd take a look). @maartenbreddels Thanks for taking a look - I'm not aware of any standard that is relevant here to be honest. I guess if you want it to be "similar to HTTP" in some sense, you might want the headers passed on in "Key: Value\n" form (with a blank line at the end...) instead of JSON, but maybe that's a bit weird for env vars really. I think the most important thing is ease of use on the Voilà (notebook) side - we shouldn't expose Voilà authors to standards just because we think they are "right" from a technical level. Maybe the easiest thing for the notebook author would be to access headers as individual env vars, e.g. X_HEADER_USER_AGENT and X_HEADER_ANYTHING_ELSE, as long as we can be consistent over naming. That would save the Voilà notebook having to JSON-decode. |
I've looked into the available standards for this (on request of @maartenbreddels) and found that CGI has specifications for HTTP headers: https://datatracker.ietf.org/doc/html/rfc3875#section-4.1.18. As previous changes (#414) to supply server information to the kernel also conformed to the CGI standard, it would make sense to continue applying those standards here too. Note that HTTP header names are case-insensitive: https://datatracker.ietf.org/doc/html/rfc2616#section-4.2, so the comparison in Would you have the time and inclination to make these changes @danlester? |
@mariobuikhuizen Hi there! In theory I would like to help, and will in due course. But not sure when I'll get on to it, so if it is important to anyone else they should please feel free to make a note here that they are working on it :) |
Thank you @danlester, I will start working on it. |
Fixed by #922 |
Similar to the QUERY_STRING environment variable that is set on a new kernel instance, this PR adds an HTTP_HEADERS env var which contains specified HTTP headers in a json-encoded dict.
There is a new configuration option, e.g. pass on the command line:
and then any new Voilà kernel will have an env var called HTTP_HEADERS containing a JSON-encoded dict which includes any of the specified headers that were available. For example:
The user-agent string could be used to provide alternative implementations or a warning message for an unsupported browser; the X-CDSDASHBOARDS-JH-USER value can be details of an authenticated user from ContainDS Dashboards (in which case, Voilà should only be accessible via the appropriate JupyterHub proxies, and not directly).
This system augments some really useful existing features or discussions such as #414 and #410.