Skip to content

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

Merged
roidelapluie merged 1 commit intoprometheus:mainfrom
FUSAKLA:fus-http-config-from-file
Dec 8, 2022
Merged

feat: make LoadHTTPConfigFile set directory and move from tests file#415
roidelapluie merged 1 commit intoprometheus:mainfrom
FUSAKLA:fus-http-config-from-file

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.

2 participants