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

feat: make LoadHTTPConfigFile set directory and move from tests file #415

Merged
merged 1 commit into from Dec 8, 2022

Conversation

FUSAKLA
Copy link
Contributor

@FUSAKLA FUSAKLA commented Nov 26, 2022

Move away the functions from the test files since they are not only test helper.
For motivation, see prometheus/prometheus#11487

Also add setting directory cfg.SetDirectory(filepath.Dir(filepath.Dir(filename))) as used in Alertmanager see
https://github.com/prometheus/alertmanager/blob/1a9f55b939ab81b86428c013ae294ad80779c9b6/cli/config/http_config.go#L36

Prometheus does not use the LoadHTTPConfigFile function AFAIK, so the change in setting the directory won't break anything, hopefully.

Other possibility is to avoid setting the directory and fix this on the Alertmanager side outside the common library.
That might be safer (function is exported, so it could be breaking change for someone, possibly?)

@roidelapluie PTAL

Signed-off-by: Martin Chodur <m.chodur@seznam.cz>
@roidelapluie
Copy link
Member

Thanks!

@FUSAKLA
Copy link
Contributor Author

FUSAKLA commented Dec 8, 2022

Thanks, @roidelapluie, for merging!

Should I wait with using of the functions in Prometheus and Alertmanager for new release/tag of this repo?
Not sure what the release lifecycle is.

@roidelapluie
Copy link
Member

roidelapluie commented Dec 8, 2022 via email

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.

None yet

2 participants