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

REST: Correct/clarify built-in-endpoints dependencies/behavior (3.10, others) #526

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

Conversation

deepkhajanchi
Copy link
Contributor

Updates in Health check endpoint

Updates in Health check endpoint
Copy link
Collaborator

@victorleeTG victorleeTG left a comment

Choose a reason for hiding this comment

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

This is not a documentation change. This is an internal technical discussion.

@@ -32,6 +32,8 @@ These endpoints are simple diagnostic utilities, which respond with the followin

.GET /echo Request and Response

Note: 9000 port is for RESTPP server then in the below request it should not return message "Hello GSQL" , do we need to correct this?
Copy link
Collaborator

Choose a reason for hiding this comment

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

You wrote an internal comment and question, not a proposed change.
Your comment is not clear: I don't know what you mean by "should" in "it should not return message 'Hello GSQL'."

Comment on lines +64 to +66
- This 'api/ping endpoint is depended on GUI service and it cannot run on the higher number of nodes than 3 based on https://github.com/tigergraph/cqrs/blob/900a0fb81a649adbb2d60884b2e5ad23bb43b2f1/tutopia/common/config/node_assignment.go#L16. Therefor, hitting this endpoint on those nodes > 3 shows 502 error. So, this should not be treated as "health check endpoint" which is confusing our customers: https://tigergraph.zendesk.com/agent/tickets/53725.
Zulip thread: https://zulip.graphtiger.com/#narrow/stream/39-TigerGraph-Frontline/topic/.2353725.3A.20.5BGoldman.20Sachs-US.5D.20-.20Need.20help.20to.20setup.20the.20paral.2E.2E.2E

Copy link
Collaborator

Choose a reason for hiding this comment

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

This again is an internal technical discussion. It seems there is some concern about the usefulness or reliability of the /api/ping endpoint. Please discuss internally to agree on under what conditions a user can safety use this endpoint and describe those conditions.

@victorleeTG victorleeTG changed the title Update built-in-endpoints.adoc REST: Correct/clarify built-in-endpoints dependencies/behavior (3.10, others) Oct 18, 2024
@victorleeTG victorleeTG added bug Something isn't working doc improv Improvements or additions to documentation labels Oct 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working doc improv Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants