Skip to content

Conversation

@tomwilkie
Copy link
Contributor

@tomwilkie tomwilkie commented Apr 7, 2020

Instead, put them in the $.mixins map.

This allows for mixins to be places in specific Grafana directories, and also removes the horrible bug around playbook links.

Still support merging into the global namespace for the time being, but this will be going away.

NB this also removes the old field names (grafana_dashboards), I couldn't find a mixin that used this anymore.

Signed-off-by: Tom Wilkie [email protected]

Instead, put them in the $.mixins map.

This allows for mixins to be places in specific Grafana directories, and also removes the horrible bug around playbook links.

Still support merging into the global namespace for the time being, but this will be going away.

Signed-off-by: Tom Wilkie <[email protected]>
Signed-off-by: Tom Wilkie <[email protected]>
@tomwilkie tomwilkie force-pushed the no-more-global-mixins branch from 7a8d3f4 to c5f5998 Compare April 7, 2020 12:18
@tomwilkie tomwilkie force-pushed the no-more-global-mixins branch from c5f5998 to 5b13029 Compare April 7, 2020 12:24
Signed-off-by: Tom Wilkie <[email protected]>
Signed-off-by: Tom Wilkie <[email protected]>
@tomwilkie tomwilkie requested a review from tombrk April 7, 2020 12:35
Copy link
Member

@tombrk tombrk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! You could add some comments to the foldr functions explaining what's their purpose.

Also I'd like to wait @malcolmholmes review, as he's currently rewriting parts of this lib as well, just to make sure these don't interfere

Signed-off-by: Tom Wilkie <[email protected]>
@tomwilkie tomwilkie mentioned this pull request Apr 7, 2020
12 tasks
@malcolmholmes
Copy link
Contributor

But the $.mixins map is still global.

You could say, the current approach is that prometheus-ksonnet "reaches out" into the global namespace for its mixins. What I am attempting to move us towards is where we create a Grafana or a "monitoring thing" and pass it our mixins:

local mymixin = import 'mymixin.libsonnet';
local grafana = import 'grafana.libsonnet';
{
  grafana: grafana.new()
     + grafana.withMixin(mymixin),
}

Which is a more conventional programming model. Because the grafana object (or monitoring thingie) holds the mixins within it, we can now have as many as we like, without them overlapping, because they have their resources handed to them, rather than the 'reaching out' that we are currently doing.

@tomwilkie
Copy link
Contributor Author

@malcolmholmes said

I don't see this as a need to block that PR

@tomwilkie tomwilkie merged commit 26cba93 into master Apr 7, 2020
@tomwilkie tomwilkie deleted the no-more-global-mixins branch April 7, 2020 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants