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

ability to setup settings #105

Merged
merged 4 commits into from
Feb 26, 2019
Merged

ability to setup settings #105

merged 4 commits into from
Feb 26, 2019

Conversation

AlexandruRus23
Copy link

I wanted to be able to configure the healthchecks in an application where I install the UI nuget package.

@unaizorrilla
Copy link
Collaborator

Hi @AlexandruRus23

Can you explain with more details which scenario you want to cover?

@AlexandruRus23
Copy link
Author

Hi @unaizorrilla

I'm running Azure Service Fabric which allocates application ports dynamically between 20000 and 30000. I'm also running Traefik as a reverse proxy. I am able to invoke Traefik's api in order to get the services' Urls and healthcheck options (virtual path of the healtcheck endpoint).

I would like to build an application hosting the Healthchecks UI via the nuget package where I pool Traefik's api in order to get the right Urls where I should perform the healthchecks. Unfortunately the static file configuration is not enough for this.

@CarlosLanderas
Copy link
Contributor

@AlexandruRus23 , I would say it would be better to bind first from configured settings and then extend with desired configuration

Instead of:

   setupSettings?.Invoke(healthCheckSettings);
   configuration.Bind(Keys.HEALTHCHECKSUI_SECTION_SETTING_KEY, healthCheckSettings);

I think is better

   configuration.Bind(Keys.HEALTHCHECKSUI_SECTION_SETTING_KEY, healthCheckSettings);
   setupSettings?.Invoke(healthCheckSettings);

@unaizorrilla
Copy link
Collaborator

Hi @AlexandruRus23

I try to review this soon, thanks for contributing.

@unaizorrilla unaizorrilla merged commit 77d5a48 into Xabaril:master Feb 26, 2019
@unaizorrilla
Copy link
Collaborator

Merged, thanks for contributing @AlexandruRus23

@AlexandruRus23
Copy link
Author

Hi @unaizorrilla
Thank you and cheers!

@unaizorrilla
Copy link
Collaborator

Please, check if all is ok for you!

@sungam3r sungam3r mentioned this pull request Jul 30, 2023
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