Skip to content
This repository has been archived by the owner on Oct 23, 2024. It is now read-only.

fixes #1106 - Return empty HealthCheck result instead of None, when no r... #1107

Merged
merged 2 commits into from
Mar 27, 2015

Conversation

drexin
Copy link
Contributor

@drexin drexin commented Jan 27, 2015

...esults are present

@sttts
Copy link
Contributor

sttts commented Jan 27, 2015

Doesn't this change the semantics of the "alive" value of the health check result? When a task has not received any health result with this patch, the /apps endpoint will return a health check result with alive=true, won't it?

@sttts
Copy link
Contributor

sttts commented Jan 27, 2015

In other words, we either have the Option[Health] type or we have to change the type of alive from Boolean to Option[Boolean].

@bobrik
Copy link
Contributor

bobrik commented Jan 27, 2015

Related to #940 too.

@drexin
Copy link
Contributor Author

drexin commented Jan 27, 2015

@sttts You're right. Closing this until I find a better solution.

@drexin drexin closed this Jan 27, 2015
@drexin
Copy link
Contributor Author

drexin commented Jan 27, 2015

Hm... Actually if it doesn't have a lastFailure set, you can be sure that it has never been healthy, so setting healthy = false should be okay.

@sttts
Copy link
Contributor

sttts commented Jan 27, 2015

Only alive is misleading. In the json we can set alive=null, in the code alive: Option[Boolean].

@drexin
Copy link
Contributor Author

drexin commented Jan 27, 2015

alive = false is correct, because it has never been alive. I think firstFailure = null should be enough to determine that it just didn't fully start yet.

@sttts
Copy link
Contributor

sttts commented Jan 27, 2015

Got what you mean. alive=false is better than healthy=false.

…he HealthCheckResult rather than its existence
@drexin drexin reopened this Jan 27, 2015
@drexin drexin added this to the 0.8.0 milestone Jan 27, 2015
@drexin drexin changed the title fixes #1106 - Return empty HealthCheck result instead on None, when no r... fixes #1106 - Return empty HealthCheck result instead of None, when no r... Jan 27, 2015
@drexin drexin modified the milestones: 0.8.1, 0.8.0 Jan 30, 2015
@sttts
Copy link
Contributor

sttts commented Feb 24, 2015

What is the opinion here?

My feeling is that the new semantics of alive=false + lastFailure=null is a bit complicated, but makes sense on the second thought.

@drexin
Copy link
Contributor Author

drexin commented Feb 25, 2015

@ConnorDoyle Could you please review?

@drexin drexin modified the milestones: 0.8.2, 0.8.1 Feb 25, 2015
kolloch pushed a commit that referenced this pull request Mar 27, 2015
fixes #1106 - Return empty HealthCheck result instead of None, when no r...
@kolloch kolloch merged commit 8e30672 into master Mar 27, 2015
kolloch pushed a commit that referenced this pull request Mar 27, 2015
8e30672
Merge: 6f1aff3 3ec8f59
Author: Peter Kolloch <[email protected]>
Date:   Fri Mar 27 16:07:26 2015 +0100

    Merge pull request #1107 from mesosphere/wip-1106-drexin

    fixes #1106 - Return empty HealthCheck result instead of None, when no r...
kolloch pushed a commit that referenced this pull request Mar 27, 2015
commit 8e30672
Merge: 6f1aff3 3ec8f59
Author: Peter Kolloch <[email protected]>
Date:   Fri Mar 27 16:07:26 2015 +0100

    Merge pull request #1107 from mesosphere/wip-1106-drexin

    fixes #1106 - Return empty HealthCheck result instead of None, when no r...
@aquamatthias aquamatthias deleted the wip-1106-drexin branch August 7, 2015 10:05
@marcomonaco marcomonaco added the pr label Mar 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants