-
Notifications
You must be signed in to change notification settings - Fork 10
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
chore: Split postgres url in to parts. #157
Conversation
Codecov Report
@@ Coverage Diff @@
## main #157 +/- ##
============================================
+ Coverage 77.28% 77.84% +0.56%
Complexity 758 758
============================================
Files 80 79 -1
Lines 3310 3300 -10
Branches 368 368
============================================
+ Hits 2558 2569 +11
+ Misses 590 569 -21
Partials 162 162
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
query-service/src/integrationTest/resources/configs/query-service/postgres/application.conf
Outdated
Show resolved
Hide resolved
query-service-impl/src/main/java/org/hypertrace/core/query/service/QueryServiceConfig.java
Outdated
Show resolved
Hide resolved
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
||
private RequestHandlerClientConfig(Config config) { | ||
this.type = config.getString(CONFIG_PATH_TYPE); | ||
this.connectionString = config.getString(CONFIG_PATH_CONNECTION_STRING); | ||
if (config.hasPath(CONFIG_PATH_USER)) { | ||
this.user = Optional.of(config.getString(CONFIG_PATH_USER)); |
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.
nit:
this.user = config.hasPath(CONFIG_PATH_USER) ? Optional.of(config.getString(CONFIG_PATH_USER)) : Optional.empty()
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.
Will do it in follow up PR
...ce-impl/src/main/java/org/hypertrace/core/query/service/postgres/PostgresResultAnalyzer.java
Show resolved
Hide resolved
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.
overall, lgtm
No description provided.