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

Health Endpoint Design Document #2503

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

sezal98
Copy link
Contributor

@sezal98 sezal98 commented Dec 30, 2024

Why make this change?

Design DOcument for Health Endpoint
Resolves: 2504

What is this change?

  • Summary of how your changes work to give reviewers context of your intent.
    • Links to relevant documentation / StackOverflow, if applicable.

How was this tested?

  • Integration Tests
  • Unit Tests

Sample Request(s)

  • Example REST and/or GraphQL request to demonstrate modifications
  • Example of CLI usage to demonstrate modifications

@abhishekkumams
Copy link
Contributor

How would the health endpoint will function with hot reload? In case of bad config it might use last good known config which might give wrong status.

Would be good to add limitations.

@sezal98
Copy link
Contributor Author

sezal98 commented Jan 3, 2025

How would the health endpoint will function with hot reload? In case of bad config it might use last good known config which might give wrong status.

Would be good to add limitations.

I dont see an issue in this, If DAB Engine is working then health point should be consistent with that. If DAB engine is using the last known config, then health endpoint would also show the health of the last known config.
Showing health of the current config but running the engine with the last known config would create inconsistencies which would be incorrect for the user.
We should put this in documentation that we are using the last known config in case of hot reload and showing health of that old config not the bad config.

Another important point to note is.. Health is independent of Validity of config. Health endpoint is only shown when the config is valid (no errors) and DAB engine is running. So the case of bad config would not occur.

@JerryNixon , Any thoughts on this?

@abhishekkumams
Copy link
Contributor

How would the health endpoint will function with hot reload? In case of bad config it might use last good known config which might give wrong status.
Would be good to add limitations.

I dont see an issue in this, If DAB Engine is working then health point should be consistent with that. If DAB engine is using the last known config, then health endpoint would also show the health of the last known config. Showing health of the current config but running the engine with the last known config would create inconsistencies which would be incorrect for the user. We should put this in documentation that we are using the last known config in case of hot reload and showing health of that old config not the bad config.

Another important point to note is.. Health is independent of Validity of config. Health endpoint is only shown when the config is valid (no errors) and DAB engine is running. So the case of bad config would not occur.

@JerryNixon , Any thoughts on this?

Thanks for clarifying. can you add the same to your documentation as well.

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

Successfully merging this pull request may close these issues.

Design Document for Health Endpoint
2 participants