-
Notifications
You must be signed in to change notification settings - Fork 59
Update Helm docs to include new configuration options of Validator Scan and Sequencer connections #3651
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
base: main
Are you sure you want to change the base?
Update Helm docs to include new configuration options of Validator Scan and Sequencer connections #3651
Conversation
martinflorian-da
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I left a fair amount of comments so maybe better to do a second iteration.
Could you also build the docs page locally and attach a screenshot? Not sure how the yaml include will look once rendered, if it will be readable well etc. Would prefer checking that before merging to avoid confusing operators.
f4e275d to
ba5ad83
Compare
[ci] Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
[ci] Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
[ci] Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
[ci] Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
[ci] Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
[ci] Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
[ci] Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
[ci] Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
[ci] Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
[ci] Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
b1d449e to
12ae26f
Compare
[ci] Signed-off-by: Pasindu Tennage <pasindu.tennage@digitalasset.com>
martinflorian-da
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! WDYT about running both a cluster test and a hdm test before merging?
|
|
||
| - Replace ``TRUSTED_SCAN_URL`` with a URL of a Scan you host or trust that is reachable by your Validator. For example, the GSF scan URL, |gsf_scan_url| | ||
| (This Scan instance will be used for obtaining additional Scan URLs for BFT Scan reads.) | ||
| You need to configure how your validator connects to the network's ``scan`` services by defining a ``scanClient`` block in your ``validator-values.yaml``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| You need to configure how your validator connects to the network's ``scan`` services by defining a ``scanClient`` block in your ``validator-values.yaml``. | |
| You need to configure how your validator connects to the network's **scan** services by defining a ``scanClient`` block in your ``validator-values.yaml``. |
The other font is for code things; IMO we just want to emphasize what this paragraph is about.
| :language: yaml | ||
| :start-after: TRUSTED_SINGLE_SCAN_START | ||
| :end-before: TRUSTED_SINGLE_SCAN_END | ||
| You need to configure how your validator's participant connects to ``sequencers`` by defining a ``synchronizer`` config in your ``validator-values.yaml``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| You need to configure how your validator's participant connects to ``sequencers`` by defining a ``synchronizer`` config in your ``validator-values.yaml``. | |
| You need to configure how your validator's participant connects to **sequencers** by defining a ``synchronizer`` config in your ``validator-values.yaml``. |
| - Expose ``/v0/holdings/summary`` endpoint from scan proxy. | ||
|
|
||
| - Add support for custom fault-tolerance configurations for ``scan`` and ``sequencer`` connections. | ||
| Please see :ref:`docs <helm-validator-install>` for details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Please see :ref:`docs <helm-validator-install>` for details. | |
| Please see the updated :ref:`documentation for Helm-based deployments <helm-validator-install>`. |
|
|
||
| - Add support for custom fault-tolerance configurations for ``scan`` and ``sequencer`` connections. | ||
| Please see :ref:`docs <helm-validator-install>` for details. | ||
| This is supported out of the box, only in the helm charts based deployment scripts. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| This is supported out of the box, only in the helm charts based deployment scripts. | |
| Docker Compose-based deployments do not currently support the new custom configuration options. |
...and I'd suggest moving that sentence to the end for better reading flow, as all the next sentences are just about Helm.
| - Add support for custom fault-tolerance configurations for ``scan`` and ``sequencer`` connections. | ||
| Please see :ref:`docs <helm-validator-install>` for details. | ||
| This is supported out of the box, only in the helm charts based deployment scripts. | ||
| New ``scanClient`` and ``synchronizer`` configs introduce a novel recommended way to configure the ``scan`` and ``sequencer`` connections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| New ``scanClient`` and ``synchronizer`` configs introduce a novel recommended way to configure the ``scan`` and ``sequencer`` connections. | |
| This introduces the new configuration keys ``scanClient`` and ``synchronizer`` as the new recommended way to configure scan and sequencer connections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe a bit nit... but I feel strongly about the "novel" (sounds odd in this context) and the font for "sequencer" and "scan"
| New ``scanClient`` and ``synchronizer`` configs introduce a novel recommended way to configure the ``scan`` and ``sequencer`` connections. | ||
| Existing configuration options ``scanAddress``, ``nonSvValidatorTrustSingleScan``, ``decentralizedSynchronizerUrl``, ``useSequencerConnectionsFromScan`` are still supported, but will be deprecated in a future release. | ||
| We recommend to migrate to the new ``scanClient`` and ``synchronizer`` configuration options as soon as possible. | ||
| Check the updated :ref:`docs <helm-validator-install>` for more details. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Check the updated :ref:`docs <helm-validator-install>` for more details. |
It's a duplicate, isn't it?
Fixes #3498