Gateway-Service Client Timeout Config#70
Conversation
aaron-steinfeld
left a comment
aaron-steinfeld
left a comment
There was a problem hiding this comment.
also, please add the default value to application.conf (as a duration, can be specified like 10s https://github.com/lightbend/config/blob/main/HOCON.md#duration-format
|
Also, welcome and thanks for the contribution! |
|
|
||
| private static final String GATEWAY_SERVICE_HOST_PROPERTY = "gateway.service.host"; | ||
| private static final String GATEWAY_SERVICE_PORT_PROPERTY = "gateway.service.port"; | ||
| private static final String GATEWAY_SERVICE_RPC_CLIENT_DEADLINE = "gateway.service.deadline"; |
There was a problem hiding this comment.
Why do we need to keep it as a mandatory field? Can we take the value from config if it is defined or take the default value?
There was a problem hiding this comment.
we do want a default, it's just a matter of where to set the default - defining it both in code and in config just makes it a bit more opaque to follow since it's an extra layer of resolution, and the config reading has to then handle the unset case.
Usually, we set defaults in application.conf, and that's optionally overriden via helm (or env vars if run via docker)
… added fallback configuration
suddendust
changed the title
aaron-steinfeld
left a comment
aaron-steinfeld
left a comment
There was a problem hiding this comment.
the snyk verification is failing due to some outdated dependencies - we can handle that separately, fine for this PR
Codecov Report
@@ Coverage Diff @@
## main #70 +/- ##
=========================================
Coverage 59.30% 59.30%
Complexity 239 239
=========================================
Files 79 79
Lines 1209 1209
Branches 34 34
=========================================
Hits 717 717
Misses 464 464
Partials 28 28
Flags with carried forward coverage won't be shown. Click here to find out more. Continue to review full report at Codecov.
|
|
Thanks @aaron-steinfeld for the quick reviews, really appreciate it. |
3 participants
Description
These changes are in context of: #69. Currently, we hardcode the deadline as 10s. Now, GATEWAY_SERVICE_DEADLINE is read from application.conf
Testing
Deployed and tested the application manually. I am still seeing if I can write a UT for check if config value is being loaded properly.
Checklist:
These changes are dependent on: hypertrace/hypertrace-service#99