Skip to content
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

Remove uber-constructor in discovery package #3014

Open
fabxc opened this Issue Aug 1, 2017 · 1 comment

Comments

Projects
None yet
3 participants
@fabxc
Copy link
Member

fabxc commented Aug 1, 2017

Currently the discovery package has one gigantic constructor for all known SD mechanism. This causes a huge dependency graph with all the SD's client libraries.
This should be refactored in a way the packages using discovery are responsible for importing SD packages directly instead.

@cstyan

This comment has been minimized.

Copy link
Contributor

cstyan commented Aug 22, 2017

This should be refactored in a way the packages using discovery are responsible for importing SD packages directly instead.
Those packages being notifier and targetmanager? And importing each SD mechanism in addition to discovery but just not calling discovery.ProvidersFromConfig

Can you confirm if the end goal is just to have those packages initialize all the SD mechanisms in the given config themselves? If so, I don't understand how that would fix the dependency issue.

hashmap added a commit to hashmap/prometheus that referenced this issue Oct 20, 2017

Move ProvidersFromConfig out of discovery package
This PR adresses prometheus#3014. In details it :
* Moves ProvidersFromConfig to a new package util/discoveryutils
* Moves discovery package tests to discovery_test package
* Introduces a new method for TargetSet

Discovery package doesn't have any dependencies from SD clients, which
makes dependency graph of new SD client much simpler.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.