hedera-json-rpc-relay icon indicating copy to clipboard operation
hedera-json-rpc-relay copied to clipboard

[DRAFT] Improvements on env variables awareness and update of the related documentation

Open Neurone opened this issue 2 years ago • 3 comments
trafficstars

Description:

Some developers have problems understanding the default configuration values used by the JSON-RPC Relay and all the possible configurations.

This PR wants to address that issue, and in particular:

  • adds a complete .env.defaults file listing all the env variables used by the application and their - explicit or computed - default values
  • review of the documentation, fixing and matching the default values with those found in the source code or in external libraries (i.e., the Hedera JavaScript SDK)

Related issue(s):

N/A

Notes for reviewer:

Still in draft, don't review.

Checklist

  • [x] Documented (Code comments, README, etc.)
  • [x] Tested (unit, integration, etc.)

Neurone avatar Oct 17 '23 21:10 Neurone

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

sonarqubecloud[bot] avatar Oct 17 '23 21:10 sonarqubecloud[bot]

@Neurone please see comments above If there's improvements to take we'd be happy to but don't want to add extra work. We could put the readme link as comment in the sample file but a file with all values might be too much

Nana-EC avatar Jan 23 '24 01:01 Nana-EC

The base idea of this PR is to relieve the burden of updating Configuration.md - now out of synchronization with reality (the code) and other parts of the documentation (i.e., the README) - and to provide security, consistency of behavior and predictability to the application and its dependencies.

The application flow should be changed like this:

  1. The application starts up and reads the default values from the .env.defaults file
  2. The application reads the .env file to override specific values
  3. The application fails if a parameter is not set; no workarounds setting default values fixed into the source code
  4. Documentation configuration tables are generated from the .env.defaults file, both for values and comments

Advantages of this flow

  • A single file guarantees developers a single point to check and update.
  • Reading from a .env.defaults file also saves the application from changes in default values in external dependencies (i.e., Hedera SDK) and can ease versioning, comparison, and backup of configurations.
  • Removing fallback values fixed in the source code makes the application more flexible, secure, and easier to write and debug.
  • Making an application fail in case of non-configured parameters is both a fixed behavior developers can implement across the project and a way to make the application more robust and prevent wrong overriding (i.e., using a file that sets a void variable)
  • Generating the documentation from the .env.defaults file can be automated (i.e., for every commit that changes that file) and give you a single point of the truth for values you can be sure the application is using.
  • Making all values explicit makes the application more robust, leaving out any doubts about what's going on or will happen.

I spoke with Alfredo today about this PR. If you like the overall idea, we can keep it, eventually with another implementation if you don't like the idea of the .env.defaults file.

If you like the idea and the solution, I can continue with this PR (sooner or later ^^) and finish it. It's not trivial or complex, I would say "doable", but it requires extensive checks nonetheless.

If you don't like the idea, we can close this.

Neurone avatar Jan 23 '24 03:01 Neurone