cmd/bosun: expr.execute refactor #1775

Merged
merged 4 commits into from Jun 9, 2016

Projects

None yet

3 participants

@kylebrandt
Member

No description provided.

@kylebrandt kylebrandt cmd/bosun: expr.execute refactor
4c6e235
@captncraig captncraig and 1 other commented on an outdated diff Jun 9, 2016
cmd/bosun/expr/expr.go
- History AlertStatusProvider
+type Contexts struct {
@captncraig
captncraig Jun 9, 2016 Contributor

+1000

@captncraig
captncraig Jun 9, 2016 Contributor

Contexts may be a bit vague. How about Backends ?

@captncraig captncraig and 1 other commented on an outdated diff Jun 9, 2016
cmd/bosun/expr/expr.go
@@ -85,26 +90,20 @@ func New(expr string, funcs ...map[string]parse.Func) (*Expr, error) {
// Execute applies a parse expression to the specified OpenTSDB context, and
// returns one result per group. T may be nil to ignore timings.
-func (e *Expr) Execute(c opentsdb.Context, g graphite.Context, l LogstashElasticHosts, eh ElasticHosts, influxConfig client.Config, cache *cache.Cache, T miniprofiler.Timer, now time.Time, autods int, unjoinedOk bool, search *search.Search, squelched func(tags opentsdb.TagSet) bool, history AlertStatusProvider) (r *Results, queries []opentsdb.Request, err error) {
+func (e *Expr) Execute(contexts *Contexts, cache *cache.Cache, T miniprofiler.Timer, now time.Time, autods int, unjoinedOk bool, search *search.Search, squelched func(tags opentsdb.TagSet) bool, history AlertStatusProvider) (r *Results, queries []opentsdb.Request, err error) {
@captncraig
captncraig Jun 9, 2016 Contributor

we still have 9 args? wow.

@kylebrandt
kylebrandt Jun 9, 2016 Member

Maybe I can do better on a revision. Will look some more as long as you approve of the direction. That func signature has been making me mad for a while :P

kylebrandt added some commits Jun 9, 2016
@kylebrandt kylebrandt also group providers
95a342a
@kylebrandt kylebrandt fix tests
5f146d8
@Dieterbe
Contributor
Dieterbe commented Jun 9, 2016

wasn't there plans to support multiple backends of a given type as well? this new design still looks a bit tightly coupled to particular implementations. (and singletons instances)

@kylebrandt
Member

nothing concrete, will be easier to do after some cleanup anyways.

@kylebrandt kylebrandt fmt
e52c937
@kylebrandt kylebrandt merged commit bb4e436 into master Jun 9, 2016

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
bosun All checks Passed!
@Dieterbe
Contributor
Dieterbe commented Jun 9, 2016

ok carry on 👍

@kylebrandt kylebrandt deleted the refactorDatasources branch Jun 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment