graphql: duplicate /health in GraphQL /admin #4768
Conversation
MichaelJCompton
added
the
area/graphql
There was a problem hiding this comment.
Reviewed 6 of 9 files at r1, 3 of 3 files at r2.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @manishrjain and @MichaelJCompton)
graphql/admin/admin.go, line 57 at r2 (raw file):
} """Node state is the state of an individual node int he Dgraph cluster """
in the
graphql/e2e/common/common.go, line 598 at r2 (raw file):
} func hasCurrentGraphQLSchema(url string) (bool, error) {
Wondering if we could just have one return parameter given how this is used.
graphql/resolve/resolver.go, line 497 at r2 (raw file):
// injectAliasCompletion takes a result with names as per the type names and swaps those for // any aliases specified in the query before apply cf.
I don't completely understand this. Do we have a test which showcases this? Is it just about returning the query result along with any aliases that are specified?
There was a problem hiding this comment.
Reviewable status: 8 of 9 files reviewed, 4 unresolved discussions (waiting on @golangcibot, @manishrjain, and @pawanrawal)
graphql/admin/admin.go, line 57 at r2 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
in the
Done.
graphql/e2e/common/admin.go, line 276 at r1 (raw file):
Previously, golangcibot (Bot from GolangCI) wrote…
G107: Potential HTTP request made with variable url (from
gosec)
Done.
graphql/e2e/common/common.go, line 598 at r2 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
Wondering if we could just have one return parameter given how this is used.
nah, we differentiate between error, false - there's no schema and true - there is a schema
graphql/resolve/resolver.go, line 497 at r2 (raw file):
Previously, pawanrawal (Pawan Rawal) wrote…
I don't completely understand this. Do we have a test which showcases this? Is it just about returning the query result along with any aliases that are specified?
Yeah, the processing that goes through Dgraph handles aliases by injecting them into the Dgraph query, but the admin functions don't have that. So they all need post processing to handle the aliases.
There was a problem hiding this comment.
Reviewable status: 8 of 9 files reviewed, 4 unresolved discussions (waiting on @golangcibot, @manishrjain, and @pawanrawal)
Note that the intention here is to keep /health, not replace it.
This change is
Docs Preview: