allow specifying network interface with "server-host" option #1280
Conversation
|
Beep boop! 🤖 Hey @nathanstitt, thanks for your PR! One of my human friends will review this PR and get back to you as soon as possible. 🕐 Stay awesome! 😎 |
|
Review app for commit 5e5c3a8 deployed to Heroku: https://hge-ci-pull-1280.herokuapp.com |
0x777
left a comment
There was a problem hiding this comment.
Thanks for the PR. Just a few changes around how the host value is stored in ServerOpts.
server/src-lib/Hasura/Server/Init.hs
Outdated
| data RawServeOptions | ||
| = RawServeOptions | ||
| { rsoPort :: !(Maybe Int) | ||
| , rsoHost :: !(Maybe String) |
There was a problem hiding this comment.
Instead of String we can directly use HostPreference (http://hackage.haskell.org/package/warp-3.2.25/docs/Network-Wai-Handler-Warp.html#t:HostPreference)
3c88c32 to
8b65ae4
Compare
|
Thanks for the review @0x777 - I've made the changes to store value as HostPreference as soon as it's read. Note that I also had to extend the |
|
Review app for commit 8b65ae4 deployed to Heroku: https://hge-ci-pull-1280.herokuapp.com |
|
This looks good! Can you document the cli flag and the environment variable here: https://github.com/hasura/graphql-engine/blob/master/docs/graphql/manual/deployment/graphql-engine-flags/reference.rst |
|
Oh neat! I had no idea those were documented. I only found the port option by grepping the code 😁 just pushed a update to docs, happy to reword if you'd like |
ca8763a to
0c5bb8d
Compare
|
Review app for commit 0c5bb8d deployed to Heroku: https://hge-ci-pull-1280.herokuapp.com |
|
Review app https://hge-ci-pull-1280.herokuapp.com is deleted |
|
Beep boop! 🤖 Whoa! 🎉 🎉 💃 Awesome work @nathanstitt! 💪 🏆 All of us at Hasura ❤️ what you did. Thanks again 🤗 |
|
Very very good. |
<!-- The PR description should answer 2 important questions: -->
### What
We want to change this error message
```json
{
"error": "error in data fusion: External error: Mutations are requested to be disallowed as part of the request"
}
```
to this
```json
{
"error": "error in data fusion: External error: Mutations are requested to be disallowed as part of the request",
"detail": {
"subgraph": "default",
"commandName": "uppercase_actor_name_by_id",
"arguments": {
"id": {
"literal": 1
}
}
}
}
```
<!-- What is this PR trying to accomplish (and why, if it's not
obvious)? -->
<!-- Consider: do we need to add a changelog entry? -->
<!-- Does this PR introduce new validation that might break old builds?
-->
<!-- Consider: do we need to put new checks behind a flag? -->
### How
<!-- How is it trying to accomplish it (what are the implementation
steps)? -->
V3_GIT_ORIGIN_REV_ID: 5d1f712c039ac9a4685c480634f1e7e17ff94c4b
Description
This adds an option to allow specifying the network address to bind to. Previously graphql-engine would bind to the wildcard interface, while I'd prefer it only listen to
localhostWhat component does this PR affect?
Requires changes from other components? If yes, please mark the components:
Related Issue
Solution and Design
I'm very much a Haskel neophyte and pretty much just copied how
server-portworked. I'm happy to rework if this could be accomplished in a better manner.Type
Checklist:
server-portis not documented. If you'd like I can add a mention for both of these to the README