allow authentication webhook with POST (close #1138) #1147
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Resolve Conflicts: docs/graphql/manual/deployment/graphql-engine-flags/reference.rst
rakeshkky
added
c/server
shahidhk
requested changes
Nov 30, 2018
|
Review app for commit 0722d1c deployed to Heroku: https://hge-ci-pull-1147.herokuapp.com |
|
Review app for commit cb07b83 deployed to Heroku: https://hge-ci-pull-1147.herokuapp.com |
shahidhk
approved these changes
Dec 3, 2018
0x777
previously approved these changes
Dec 3, 2018
|
@rikinsk Need your approval on docs. |
shahidhk
requested changes
Dec 3, 2018
|
Review app for commit 1d8568b deployed to Heroku: https://hge-ci-pull-1147.herokuapp.com |
|
Review app for commit a824638 deployed to Heroku: https://hge-ci-pull-1147.herokuapp.com |
0x777
previously approved these changes
Dec 3, 2018
shahidhk
previously approved these changes
Dec 3, 2018
shahidhk
approved these changes
Dec 3, 2018
|
Review app for commit 553b855 deployed to Heroku: https://hge-ci-pull-1147.herokuapp.com |
|
Review app https://hge-ci-pull-1147.herokuapp.com is deleted |
hasura-bot
pushed a commit
that referenced
this pull request
Sep 24, 2024
<!-- The PR description should answer 2 important questions: --> ### What This PR adds the `Spanned` type: an OpenDD wrapper that can be placed inside the Metadata. It's basically a pair of the value and the path to the value in the original metadata. This allows us to do things like source maps. <!-- What is this PR trying to accomplish (and why, if it's not obvious)? --> <!-- Consider: do we need to add a changelog entry? --> <!-- Does this PR introduce new validation that might break old builds? --> <!-- Consider: do we need to put new checks behind a flag? --> ### How It's almost exactly what @danieljharvey proposed originally. <!-- How is it trying to accomplish it (what are the implementation steps)? --> --------- Co-authored-by: Daniel Harvey <[email protected]> V3_GIT_ORIGIN_REV_ID: 4f037686b6981fffc4b0a8ac8f95c2f9c623af67
hasura-bot
pushed a commit
that referenced
this pull request
Sep 25, 2024
<!-- The PR description should answer 2 important questions: --> ### What We would like to show error paths for `metadata-resolve` so that debugging these errors is a little less painful, both for us and end users. To this end, #1147 introduced a type wrapper that would be deserialised to contain its own JSON path, so we could then pass this path to errors. This PR does precisely this for the `UnknownModelDataConnector` error. I chose this error because... it was the first one on the list, not for any reason beyond that. Right now, this is an extremely simple case whereby only one path is required, however other errors may need two ("name at path X conflicts with name at path Y", for example). This PR also changes the default engine error stdout to show the `Debug` instance rather than the `Display` instance, as the error path is discarded by the `Display` instance. Unfortunately, we use `Display` for both stdout and user responses, which is maybe something we'd want to change eventually, but for now this means we can't just add the error path to the `Display` instance. ### How I started by making `Model` a `Spanned` element within the metadata structure. I then added the `path` key to the resolved `Model` type. I then found the first error type that included a model name, and added the `path` key to that error variant. Then, I just did the wiring. You'll note that this error doesn't _alway_ return a path because it isn't always raised by a model-first code path, but this is probably the first PR of many. ### Next steps * Next step is to make the output a little neater, probably by creating an actual structured error type (most likely a lot like `Spanned`, with a `path` and a `value`). Then, we can use a `Display` instance again to print this nicely in the stdout, but ignore the path in the MBS API response. * After that, the plan is to stop ignoring it in the MBS API response, with a new key to hold an error path. * Step three is to allow for errors to produce multiple error paths in a list, hopefully such that they tell a story ("I found this... but then I found this... and those two things conflict") * Step four will be a wave of PRs that look quite similar to this one, wiring up paths to as many errors as possible. <!-- How is it trying to accomplish it (what are the implementation steps)? --> V3_GIT_ORIGIN_REV_ID: 2d8dda018055f65711e66b08aa15188b516e2ddc
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
What component does this PR affect?
Requires changes from other components? If yes, please mark the components:
Related Issue
close #1138
Solution and Design
Use
--auth-hook-modeoption orHASURA_GRAPHQL_AUTH_HOOK_MODEenv variable to set authentication webhook mode (
POSTorGET). Default isGETHasura's request to auth hook:-
Type
Checklist: