bitflicker64 commented on code in PR #529: URL: https://github.com/apache/hugegraph-doc/pull/529#discussion_r4283747927
########## content/en/docs/guides/hugegraph-docker-cluster.md: ########## @@ -209,6 +209,8 @@ Unlike PD and Store, Server has no mandatory entrypoint variables: only set valu | `HG_SERVER_USE_PD` | `false` | `usePD` in `rest-server.properties` | | `HG_SERVER_CLUSTER` | `hg-test` | `cluster` in `rest-server.properties` | | `HG_SERVER_REST_URL` | `http://0.0.0.0:8080` (set in the image) | `restserver.url` | +| `HG_SERVER_URLS_TO_PD` | `http://0.0.0.0:8080` | `server.urls_to_pd`: comma-separated URLs this Server registers in PD instead of `restserver.url`; read only when `server.deploy_in_k8s` is `true`; whitespace is removed | Review Comment: Minor: This row does not say that every URL in the list needs its own `http://` scheme. `server.urls_to_pd` is declared with `withUrlNormalization("http://";)`, and `HugeConfig.prefixSchemeIfMissing` treats the whole value as one string, so only the first entry gets a scheme. `GraphManager` then splits on `,` and registers the rest as written: `hg-0:8080,hg-1:8080` registers `http://hg-0:8080` and `hg-1:8080`. The README added in apache/hugegraph#3352 tells users to give each URL its own `http://` prefix. Please add the same note here and to the CN row at line 212. ########## content/en/docs/config/config-guide.md: ########## @@ -36,7 +36,7 @@ The file-loading behavior on this page was checked against Server master commit Complete defaults are in Server master: [gremlin-server.yaml](https://github.com/apache/hugegraph/blob/master/hugegraph-server/hugegraph-dist/src/assembly/static/conf/gremlin-server.yaml), [rest-server.properties](https://github.com/apache/hugegraph/blob/master/hugegraph-server/hugegraph-dist/src/assembly/static/conf/rest-server.properties), [hugegraph.properties](https://github.com/apache/hugegraph/blob/master/hugegraph-server/hugegraph-dist/src/assembly/static/conf/graphs/hugegraph.properties), and [hstore.properties.template](https://github.com/apache/hugegraph/blob/master/hugegraph-server/hugegraph-dist/src/assembly/static/conf/graphs/hstore.properties.template). -The following Docker environment-variable behavior applies only to images built from `master`; arbitrary variable names are not converted into configuration options. Before initialization and startup, this entrypoint processes `HG_SERVER_BACKEND`, `HG_SERVER_PD_PEERS`, `HG_SERVER_USE_PD`, `HG_SERVER_CLUSTER`, `HG_SERVER_REST_URL`, `HG_SERVER_MIN_FREE_MEMORY`, `HG_SERVER_AUTH_TOKEN_SECRET`, `HG_SERVER_INIT_STORE_ENABLED`, and `PASSWORD`. Check the tagged entrypoint for historical release images. See [Docker entrypoint](https://github.com/apache/hugegraph/blob/master/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh). +The following Docker environment-variable behavior applies only to images built from `master`; arbitrary variable names are not converted into configuration options. Before initialization and startup, this entrypoint processes `HG_SERVER_BACKEND`, `HG_SERVER_PD_PEERS`, `HG_SERVER_USE_PD`, `HG_SERVER_CLUSTER`, `HG_SERVER_REST_URL`, `HG_SERVER_URLS_TO_PD`, `HG_SERVER_DEPLOY_IN_K8S`, `HG_SERVER_MIN_FREE_MEMORY`, `HG_SERVER_AUTH_TOKEN_SECRET`, `HG_SERVER_INIT_STORE_ENABLED`, and `PASSWORD`. Check the tagged entrypoint for historical release images. See [Docker entrypoint](https://github.com/apache/hugegraph/blob/master/hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh). Review Comment: Important: This sentence, and the two new rows in `hugegraph-docker-cluster.md` (EN and CN), say the `master` entrypoint already reads `HG_SERVER_URLS_TO_PD` and `HG_SERVER_DEPLOY_IN_K8S`. That mapping exists only in apache/hugegraph#3352, which is still open. On apache/hugegraph master e130233c, `hugegraph-server/hugegraph-dist/docker/docker-entrypoint.sh` does not reference either variable, and line 177 still holds the TODO to map them. If this page is published first, a user who sets either variable gets no error, and the Server keeps registering `restserver.url` in PD. Please hold this PR until #3352 is merged (the description already asks for the two to merge together), then re-check the wording against the merged entrypoint. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
