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

SE and MP Health Guides #925

Merged
merged 22 commits into from
Aug 29, 2019
Merged

SE and MP Health Guides #925

merged 22 commits into from
Aug 29, 2019

Conversation

pfmackin
Copy link
Contributor

@pfmackin pfmackin commented Aug 20, 2019

This is the first pass at a new SE and MP Health Guide

Copy link
Contributor

@romain-grecourt romain-grecourt left a comment

Choose a reason for hiding this comment

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

LGTM, can you also add a new card in the guides overview ?

@pfmackin
Copy link
Contributor Author

pfmackin commented Aug 20, 2019

I can do that but there was some discussion between Joe and Mark about refactoring the structure. Anyway, I will do that for now.

This is done

@romain-grecourt
Copy link
Contributor

Thanks, at least the guide can fit nicely until we re-work the guides overview.

@romain-grecourt
Copy link
Contributor

Couple of thoughts.

The guide seems a little long, maybe we can cut the length by removing the JSON snippets for the expected outputs.

I think we should update the quickstart YAML file to add what's missing and documented in this guide (separate PR).

Note that I'm working on a bare archetype, when that's ready we can update this guide (separate) to use it.

@markxnelson
Copy link
Contributor

I don't think it is too long - I think some of the other existing ones are too short - they leave out vital information that a person who is not already familiar with Helidon (or MP) would actually need imho.

@pfmackin
Copy link
Contributor Author

I can do whatever you folks want regarding the JSON. I personally like a guide to be something that a user can cut and paste into and IDE, build, run, and verify that the output is correct.

@romain-grecourt
Copy link
Contributor

There aren't many other guides to compare the length to. Quickstart and meant to be "quick", the other ones are not really comparable.

The current angle for the guides is less to explain and more to exercise ; we should try to avoid overlapping with the "normal" docs.

@pfmackin pfmackin changed the title SE Health Guide SE and MP Health Guides Aug 22, 2019
@m0mus
Copy link
Contributor

m0mus commented Aug 23, 2019

Most my comments for MP guide are valid for SE guide as well. Please fix it in both guides.

docs/src/main/docs/guides/07_health_mp_guide.adoc Outdated Show resolved Hide resolved
docs/src/main/docs/guides/07_health_mp_guide.adoc Outdated Show resolved Hide resolved
docs/src/main/docs/guides/07_health_mp_guide.adoc Outdated Show resolved Hide resolved
docs/src/main/docs/guides/07_health_mp_guide.adoc Outdated Show resolved Hide resolved
docs/src/main/docs/guides/07_health_mp_guide.adoc Outdated Show resolved Hide resolved
docs/src/main/docs/guides/07_health_mp_guide.adoc Outdated Show resolved Hide resolved
@m0mus
Copy link
Contributor

m0mus commented Aug 23, 2019

@romain-grecourt A guide should contain enough to get started with some specific topic without looking on normal docs. It may contain links to documentation if users want to go deeper in the topic.

@romain-grecourt
Copy link
Contributor

@m0mus I disagree, but there is no need to argue any further.

:description: Helidon health-checks
:keywords: helidon, health-checks, health, check

This document describes basic examples that use both built-in and custom health-checks with Helidon MP.
Copy link
Contributor

Choose a reason for hiding this comment

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

This tutorial describes how to create a sample MicroProfile (MP) project that can be used to run some basic examples using both built-in and custom health-checks with Helidon MP.

* available disk space
* available heap memory

This example will demonstrate how to use the built-in health-checks. These examples are all executed
Copy link
Contributor

Choose a reason for hiding this comment

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

The following example will

The example below will change the root path.

[source,yaml]
.Create a file named application.yaml in the resources directory with the following contents
Copy link
Contributor

Choose a reason for hiding this comment

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

with the following contents:

@barchetta
Copy link
Member

@pfmackin currently the health wire response provides both outcome/state and status. Outcome and state were replaced by status in MicroProfile Health 2.0. So use of outcome and state are deprecated. I think this should be mentioned in the guide. Maybe a tip or note like:

In MicroProfile Health 2.0 outcome and state were replaced by status in the JSON response wire format. Helidon currently provides both fields for backwards compatibility, but use of outcome and state is deprecated and will be removed in a future release. You should rely on state instead.

I think this might also apply to Helidon SE health. You might want to confirm with @tomas-langer

@pfmackin
Copy link
Contributor Author

I fixed this in MP. SE doesn't return status, it just returns state and outcome.

Copy link
Contributor

@m0mus m0mus left a comment

Choose a reason for hiding this comment

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

LGTM

@barchetta barchetta merged commit 6e8f13c into helidon-io:master Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants