-
Notifications
You must be signed in to change notification settings - Fork 30
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
Strongly typed Datastore configuration - Fixes #2088 #2223
Conversation
1700a29
to
e12720c
Compare
...src/main/java/io/hyperfoil/tools/horreum/api/data/datastore/CollectorApiDatastoreConfig.java
Outdated
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.
I added several comments to the code, but overall looks good.
...src/main/java/io/hyperfoil/tools/horreum/api/data/datastore/CollectorApiDatastoreConfig.java
Outdated
Show resolved
Hide resolved
horreum-api/src/main/java/io/hyperfoil/tools/horreum/api/data/datastore/DatastoreType.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/hyperfoil/tools/horreum/api/data/datastore/ElasticsearchDatastoreConfig.java
Outdated
Show resolved
Hide resolved
horreum-backend/src/main/java/io/hyperfoil/tools/horreum/svc/ConfigServiceImpl.java
Outdated
Show resolved
Hide resolved
horreum-backend/src/main/java/io/hyperfoil/tools/horreum/svc/ConfigServiceImpl.java
Outdated
Show resolved
Hide resolved
horreum-web/src/domain/admin/datastore/ModifyDatastoreModal.tsx
Outdated
Show resolved
Hide resolved
horreum-web/src/domain/admin/datastore/ModifyDatastoreModal.tsx
Outdated
Show resolved
Hide resolved
94aecb4
to
de69cbb
Compare
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.
Added some suggestions for more cleanup on the API module
...src/main/java/io/hyperfoil/tools/horreum/api/data/datastore/CollectorApiDatastoreConfig.java
Show resolved
Hide resolved
...src/main/java/io/hyperfoil/tools/horreum/api/data/datastore/CollectorApiDatastoreConfig.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/hyperfoil/tools/horreum/api/data/datastore/ElasticsearchDatastoreConfig.java
Show resolved
Hide resolved
...rc/main/java/io/hyperfoil/tools/horreum/api/data/datastore/ElasticsearchDatastoreConfig.java
Show resolved
Hide resolved
...src/main/java/io/hyperfoil/tools/horreum/api/data/datastore/CollectorApiDatastoreConfig.java
Show resolved
Hide resolved
...api/src/main/java/io/hyperfoil/tools/horreum/api/data/datastore/PostgresDatastoreConfig.java
Show resolved
Hide resolved
...api/src/main/java/io/hyperfoil/tools/horreum/api/data/datastore/PostgresDatastoreConfig.java
Show resolved
Hide resolved
...rc/main/java/io/hyperfoil/tools/horreum/api/data/datastore/ElasticsearchDatastoreConfig.java
Outdated
Show resolved
Hide resolved
...rc/main/java/io/hyperfoil/tools/horreum/api/data/datastore/ElasticsearchDatastoreConfig.java
Outdated
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.
LGTM!
I added just a minor comment, up to you to include it or not as it would be just a simplification 🚀
horreum-backend/src/main/java/io/hyperfoil/tools/horreum/datastore/CollectorApiDatastore.java
Outdated
Show resolved
Hide resolved
- Updates from review - Update datastore config checks touse isBlank() - Update docs
I think this one is good to go. |
This is a fairly major re-factor of the
Datastore
configuration API.It solves the following problems;
No Auth
,Api Key
andUsername/Password