Configure HTTP_PORT and WS_PORT via environment variables #2832
Conversation
node/src/opt.rs
Outdated
| @@ -124,6 +125,7 @@ pub struct Opt { | |||
| long, | |||
| default_value = "8020", | |||
| value_name = "PORT", | |||
| env = "WS_PORT", | |||
There was a problem hiding this comment.
This seems wrongly placed. This is for the JSON-RPC of the admin API ...
There was a problem hiding this comment.
you're looking for the one with a default of 8001. Also, how does this change the graphiql web ui to use the new port?
There was a problem hiding this comment.
Looks like the port is divined from configuration, so it should work fine once this points to the right option: https://github.com/graphprotocol/graph-node/blob/master/server/http/src/service.rs#L120
There was a problem hiding this comment.
@numonedad it looks like the HTTP_PORT is possibly taken care of in the graphiql web ui by using window.location.pathname here so I presumably just need to worry about the WS_PORT for the ui?
There was a problem hiding this comment.
I think it's passed along fine, we just need a way to configure WS_PORT when starting graph_node
node/src/opt.rs
Outdated
| @@ -110,6 +110,7 @@ pub struct Opt { | |||
| long, | |||
| default_value = "8030", | |||
| value_name = "PORT", | |||
| env = "HTTP_PORT", | |||
|
@Charles-Johnson Are you going to update your PR? |
|
Sorry @maoueh , I've been occupied by work and family for the last 2 weeks. I need still need to find some time to understand the proper fix |
|
@Charles-Johnson Didn't wanted to put pressure here, sorry if it felt that way. Just wanted to ensure this was still active :) |
|
Sorry I realised that I edited the wrong macro 😅 . It's a little confusing because there's no spatial grouping of the macro and struct field definition. Anyway I will update |
Charles-Johnson
force-pushed
the
Charles-Johnson-fix-for-#2829
branch
from
1586adb to
f76e6cc
Compare
|
Sorry for such long delay, completely forgot. Would you have time to change |
|
@Charles-Johnson @maoueh will this be continued? Otherwise I would "vote" for closing it. |
|
@otaviopace I'll take care of bringing it in. |
|
@maoueh just looking into the quick list items now |
merge latest changes from master and document environment variables
|
@maoueh I made the requested changes |
There was a problem hiding this comment.
Looks like the review was addressed, thanks @Charles-Johnson
This should resolve issue #2829.