-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 context to dependencies endpoint #2434
Add context to dependencies endpoint #2434
Conversation
771b6c5
to
f322f40
Compare
/dependencies
endpointThere 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.
Nice change! I left a few minor comments.
@@ -213,7 +213,7 @@ var _escData = map[string]*_escFile{ | |||
name: "index.html", |
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.
please exclude this file from the PR, it's not related
indices := getIndices(s.indexPrefix, endTs, lookback) | ||
searchResult, err := s.client.Search(indices...). | ||
Size(10000). // the default elasticsearch allowed limit | ||
Query(buildTSQuery(endTs, lookback)). | ||
IgnoreUnavailable(true). | ||
Do(s.ctx) | ||
Do(ctx) |
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.
Good catch. Can we delete the field s.ctx
altogether? I think it was implemented as anti-pattern.
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.
deleted
@@ -213,7 +213,7 @@ var _escData = map[string]*_escFile{ | |||
name: ".nocover", |
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.
please exclude this file
f322f40
to
86dc478
Compare
Signed-off-by: Yoav Eyal <[email protected]>
86dc478
to
614701d
Compare
Signed-off-by: Yoav Eyal <[email protected]>
Signed-off-by: yoave23 <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #2434 +/- ##
=======================================
Coverage 95.57% 95.58%
=======================================
Files 208 208
Lines 10682 10681 -1
=======================================
Hits 10209 10209
+ Misses 400 399 -1
Partials 73 73
Continue to review full report at Codecov.
|
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.
Thanks!
Which problem is this PR solving?
/dependencies
endpointShort description of the changes
context.Context
param to theGetDependencies
interface and implement accordingly