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

Eureka health status #78

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

michl-b
Copy link
Contributor

@michl-b michl-b commented Sep 28, 2017

No description provided.

@danielbayerlein
Copy link
Owner

@michl-b Thank you for your PR. We'll check it as soon as possible. 👍

Copy link
Owner

@danielbayerlein danielbayerlein left a comment

Choose a reason for hiding this comment

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

Thank you for your work! 💚

README.md Outdated

#### props

* `title`: Widget title (Default: `GitHub Issue Count`)
Copy link
Owner

Choose a reason for hiding this comment

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

TODO: Default title -> Eureka Health Status

README.md Outdated
#### props

* `title`: Widget title (Default: `GitHub Issue Count`)
* `interval`: Refresh interval in milliseconds (Default: `300000`)
Copy link
Owner

Choose a reason for hiding this comment

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

TODO: Default -> 3600000

README.md Outdated
* `interval`: Refresh interval in milliseconds (Default: `300000`)
* `url`: Eureka Server Base URL
* `healthQuery`: Relative Path to Spring Boot Actuator Health endpoint
* `appsQuery`: Relative Path to Eureka Apps API endpoint [Eureka REST operations](https://github.com/Netflix/eureka/wiki/Eureka-REST-operations)
Copy link
Owner

Choose a reason for hiding this comment

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

TODO: Remove double whitespace

README.md Outdated
* `title`: Widget title (Default: `GitHub Issue Count`)
* `interval`: Refresh interval in milliseconds (Default: `300000`)
* `url`: Eureka Server Base URL
* `healthQuery`: Relative Path to Spring Boot Actuator Health endpoint
Copy link
Owner

Choose a reason for hiding this comment

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

TODO: Documentation for baseQuery is missing

README.md Outdated
* `appsQuery`: Relative Path to Eureka Apps API endpoint [Eureka REST operations](https://github.com/Netflix/eureka/wiki/Eureka-REST-operations)
* `authKey`: Credential key, defined in [auth.js](./auth.js)
* `appNamePattern`: Name pattern the service-names have to start with
* `minimumInstances`: Number of instances for each service which are expected to run to be fine
Copy link
Owner

Choose a reason for hiding this comment

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

TODO: Documentation for minimumInstances default value is missing

}
}

if (hasError === false) {
Copy link
Owner

Choose a reason for hiding this comment

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

TODO: if (!hasError) {

appStatus += ` - ${appsStatus[2]}: ${appsStatus[3]}`
}

if (hasError === false) {
Copy link
Owner

Choose a reason for hiding this comment

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

TODO: if (!hasError) {

let infoMessage = ''
let hasError = json.status !== 'UP'

if (hasError === false) {
Copy link
Owner

Choose a reason for hiding this comment

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

TODO: if (!hasError) {

appStatus = `${appsStatus[0]}: ${appsStatus[1]}`
} else {
appStatus = `${appsStatus[0]}: ${appsStatus[1]}`
appStatus += ` - ${appsStatus[2]}: ${appsStatus[3]}`
Copy link
Owner

Choose a reason for hiding this comment

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

TODO: You can use a single line

appStatus = `${appsStatus[0]}: ${appsStatus[1]} - ${appsStatus[2]}: ${appsStatus[3]}`

return hasError
}

async checkInstanceHealth (url, appNamePattern, appList) {
Copy link
Owner

Choose a reason for hiding this comment

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

I think we can optimize this function in the next step. @chrishelgert Can you support @michl-b, please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants