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

Blacklisted Healthcheck types #120

Merged
merged 1 commit into from
Sep 13, 2016
Merged

Conversation

dankraw
Copy link
Contributor

@dankraw dankraw commented Sep 12, 2016

Introducing consul-ignored-healthchecks configuration property which allows one to ignore specified types of Marathon health checks during service registration in Consul.

Fixes #118

@coveralls
Copy link

coveralls commented Sep 12, 2016

Coverage Status

Coverage increased (+0.2%) to 79.276% when pulling a1b6468 on blacklisted-healthcheck-types into f4d4a70 on develop.

}

func (c *Consul) getIgnoredHealthCheckTypes() []string {
ignoredTypes := make([]string, 0)
Copy link
Contributor

@janisz janisz Sep 12, 2016

Choose a reason for hiding this comment

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

How about something like this:

ignoredTypes := strings.Split(strings.ToUpper(c.config.IgnoredHealthChecks), ","))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I've fixed it so ToUpper is called once, but still I'd like to check if the input is valid (trim, remove empty values):

ignoredTypes := make([]string, 0)
    for _, ignoredType := range strings.Split(strings.ToUpper(c.config.IgnoredHealthChecks), ",") {
        var ignoredType = strings.TrimSpace(ignoredType)
        if ignoredType != "" {
            ignoredTypes = append(ignoredTypes, ignoredType)
        }
    }
    return ignoredTypes

By default it returns empty splice.

@druminski
Copy link

👍

@dankraw dankraw force-pushed the blacklisted-healthcheck-types branch from a1b6468 to 05376d4 Compare September 13, 2016 06:51
@coveralls
Copy link

coveralls commented Sep 13, 2016

Coverage Status

Coverage increased (+0.2%) to 79.276% when pulling 05376d4 on blacklisted-healthcheck-types into f4d4a70 on develop.

@dankraw dankraw force-pushed the blacklisted-healthcheck-types branch from 05376d4 to 0285ada Compare September 13, 2016 09:19
@coveralls
Copy link

coveralls commented Sep 13, 2016

Coverage Status

Coverage increased (+0.2%) to 79.206% when pulling 0285ada on blacklisted-healthcheck-types into f4d4a70 on develop.

@janisz
Copy link
Contributor

janisz commented Sep 13, 2016

LGTM 👍

@dankraw dankraw merged commit 83ca712 into develop Sep 13, 2016
@dankraw dankraw deleted the blacklisted-healthcheck-types branch September 13, 2016 12:03
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.

4 participants