go-marathon icon indicating copy to clipboard operation
go-marathon copied to clipboard

Published 20 hours ago • verified publisher icon gambol99

Adds delaySeconds field to healthchecks

Open cihangirbesiktas opened this issue 7 years ago • 5 comments

cihangirbesiktas avatar Aug 03 '17 10:08 cihangirbesiktas

The tests are currently failing.

timoreimann avatar Aug 11 '17 01:08 timoreimann

Removed residency since it is on another PR and made the tests pass. Could you approve and merge @timoreimann ?

cihangirbesiktas avatar Aug 14 '17 12:08 cihangirbesiktas

@cihangirbesiktas as expressed by myself and @marco-jantke, extending NewDefaultHealthCheck doesn't seem necessary: We apparently gain nothing nothing from it for now as we chose the current Marathon default value for delaySeconds, but we risk diverging from potential changes that may happen in the future.

If you think there's a good reason to keep it, I'd be grateful for an explanation. Otherwise, I'd prefer to see that part getting removed.

timoreimann avatar Aug 14 '17 19:08 timoreimann

We should have one small test somewhere that verifies we can properly deserialize the test data JSON that you have extended by the new field.

timoreimann avatar Aug 15 '17 10:08 timoreimann

What kind of test are you expecting?

cihangirbesiktas avatar Nov 20 '17 17:11 cihangirbesiktas