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

Wrong health check object generated for COMMAND protocol #222

Closed
protetore opened this issue Oct 16, 2017 · 3 comments
Closed

Wrong health check object generated for COMMAND protocol #222

protetore opened this issue Oct 16, 2017 · 3 comments

Comments

@protetore
Copy link

protetore commented Oct 16, 2017

The current version of marathon-python (0.9.2) generates COMMAND type protocol healthchecks command property as a string, but current version of marathon expects an object with the 'value' property:

Current:

{
  "protocol": "COMMAND",
  "command":  "curl -f -X GET http://$HOST:$PORT0/health"
}

Expected:

{
  "protocol": "COMMAND",
  "command": { "value": "curl -f -X GET http://$HOST:$PORT0/health" }
}

My workaround was to create the health check like this:

healthchecks.append(MarathonHealthCheck(
    protocol=hc.protocol,
    command={"value": hc.cmd}
))

I can make a pull request to address this, but there are some questions:

  1. This should be fixed or we should assume the we have to send the object in the correct format? (:-1:)
  2. This should be fixed, but there must be a quick test to see if the received command is already an object to avoid breaking the code of those who are creating the like I am?
  3. Just change the code to create the format expected by marathon and let people fix if they bump into some error?
@solarkennedy
Copy link
Contributor

@protetore can you track down exactly when this behavior changed in Marathon?

@protetore
Copy link
Author

I've checked in oldest tag I could find in Marathon repo (1.1.1) and it already required the nested value object.

Then I went to the oldest release branch (release/0.13) and it was already there too.

In their words: "command health checks WITHOUT a nested value should be rejected"

@solarkennedy
Copy link
Contributor

Yea I guess at Yelp we've been passing in the value dictionary.

Please do your solution number 2 and print a deprecation error.

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

No branches or pull requests

2 participants