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

[skip-ci] Service Status RFC #59621

Merged
merged 14 commits into from
Mar 16, 2020
Merged

Conversation

joshdover
Copy link
Contributor

@joshdover joshdover commented Mar 8, 2020

Summary

RFC for #41983

View rendered

@joshdover joshdover added Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes RFC v7.7.0 labels Mar 8, 2020
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-platform (Team:Platform)

@joshdover joshdover force-pushed the rfc/service-status branch from c6c5534 to 497ff50 Compare March 8, 2020 23:07
@joshdover joshdover requested a review from a team March 9, 2020 14:35
rfcs/text/0008_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0008_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0008_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0008_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0008_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0008_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0008_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0008_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0008_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0008_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0010_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0010_service_status.md Show resolved Hide resolved
rfcs/text/0010_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0010_service_status.md Show resolved Hide resolved
rfcs/text/0010_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0010_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0010_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0010_service_status.md Show resolved Hide resolved
rfcs/text/0010_service_status.md Outdated Show resolved Hide resolved
- Add clarity about which Core services are included in `CoreStatus`
- Add JSON-serializable types for `meta` property
- Add overall status calculation section
- Add note that some plugin contract APIs may throw
- Add `self` argument to `unavailableWhen` utility
- Remove unresolved questions that have been addressed
@joshdover
Copy link
Contributor Author

joshdover commented Mar 11, 2020

Moving a little aggressively on this one, but I'm going to go ahead and put this RFC in the final comment period. If there are any fundamental issues with this proposal, please raise ASAP.

If no fundamental problems are found, this RFC will be accepted and merged on Monday, March 16.

@joshdover joshdover added the RFC/final-comment-period If no concerns are raised in 3 business days this RFC will be accepted label Mar 11, 2020
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

Makes sense to me. I think this will be a nice evolution of the legacy API.

Added a couple notes, but they are all minor

rfcs/text/0010_service_status.md Show resolved Hide resolved
rfcs/text/0010_service_status.md Show resolved Hide resolved
rfcs/text/0010_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0010_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0010_service_status.md Outdated Show resolved Hide resolved
rfcs/text/0010_service_status.md Show resolved Hide resolved
rfcs/text/0010_service_status.md Show resolved Hide resolved
Copy link
Contributor

@rudolf rudolf left a comment

Choose a reason for hiding this comment

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

Hooray for resilient error handling!

rfcs/text/0010_service_status.md Outdated Show resolved Hide resolved
/**
* The current availability level of the service.
*/
level: ServiceStatusLevel.available;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't that be just ServiceStatusLevel ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This union type is to express that when level is not available, the summary field is required. This first part of the union is the case where it is not required.

rfcs/text/0010_service_status.md Outdated Show resolved Hide resolved
@joshdover joshdover merged commit 7fa5c2f into elastic:master Mar 16, 2020
@joshdover joshdover deleted the rfc/service-status branch March 16, 2020 15:24
@joshdover joshdover added the backport:skip This commit does not require backporting label Mar 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting Feature:New Platform release_note:skip Skip the PR/issue when compiling release notes RFC/final-comment-period If no concerns are raised in 3 business days this RFC will be accepted RFC Team:Core Core services & architecture: plugins, logging, config, saved objects, http, ES client, i18n, etc v7.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants