-
Notifications
You must be signed in to change notification settings - Fork 31
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
Add Collector API datastore #1703
Conversation
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.
One thing we need to think about is duplicated runs. As it currently works, the call to the collector api simply returns all the results for a given tag
, each invocation will return all the data and will be uploaded into Horreum
So, we either need to be able to query the api to return a subset of the data, or we need to handle de-duplication in Horreum
|
||
HttpClient client = HttpClient.newHttpClient(); | ||
HttpRequest.Builder builder = HttpRequest.newBuilder() | ||
.uri(java.net.URI.create(jsonDatastoreConfig.url + "/api/v1/image-stats/tag/" + payload.get("tag").asText())); |
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.
should the url path
be a separate config param, or part of the jsonDatastoreConfig.url
, if the api changes (e.g. a version bump) then this hard-coded path will likely be no longer valid
Ah, I now see this issue wrt filtering the api result: Karm/collector#34 |
Karm/collector#35 is now merged enabling filtering by tag, benchmark, and date range (not pushed to production yet though). So I will adapt the PR to use the new API. |
@zakkak excellent news. |
6f92ebe
to
7ba3fe7
Compare
7ba3fe7
to
a54ed3a
Compare
@johnaohara this should now be ready for review. |
a54ed3a
to
910a861
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.
LGTM!
@zakkak Many thanks for your contribution! |
Fixes Issue
Closes #1109
Proposed Changes
This patch issues a request to the collector API to get all available data. To limit the size of returned data we require the use of a specific
tag
andimgName
combination, while also limiting the date range.Check List (Check all the applicable boxes)