mirror of
https://github.com/hasura/graphql-engine.git
synced 2024-12-15 17:31:56 +03:00
b4f7e96665
### Motivation While we strive to write clear code, we have historically struggled at Hasura with having very different styles and standards across the codebase. There's been efforts to standardize our coding style, we have an official styleguide that isn't maintained as closely as it should... We still have some work in front of us. However, in the last ~year or so, there's been a huge push towards incrementally improving the situation. As part of this we've been blocking PRs that don't add enough comments, or don't improve the files that they touch. While looking at `Hasura.GraphQL.Analyse`, it became apparent that this file did not meet the engineering standards that I would expect to see addressed during a code review. Some ways in which I think it falls short: - lack of documentation - no clear distinction between public / internal components - "unidiomatic" Haskell code (such as using `Either Result Error`) While there's no problem with a file looking like this during development, those issues should have been caught at review time. The fact that they weren't indicates a problem in our process that we will need to address: code quality and maintainability is paramount, and we all need to do our part. ### Description This PR rewrites all of `Hasura.GraphQL.Analyze`, and adapts `Hasura.Server.OpenAPI` accordingly where needed. I've attempted to clarify names and add documentation based on my understanding of the code, and to clean what was unused (such as field variables). I don't think this PR is good enough as is, and I welcome criticism where I got my comments wrong / am happy to help y'all add more. This PR makes one small change in the way error messages are reported (and adjusts the corresponding test accordingly); each error message is now prefixed with the path within the selection set: ``` ⚠️ $.test.foo.bar.baz.mizpelled: field 'mizpelled' not found in object 'Baz' ``` ### Note This PR is currently **on top of #3962**. You can preview the changes in isolation by [diffing the branches](https://github.com/hasura/graphql-engine-mono/compare/nicuveo/clean-rest-endpoint-inconsistency-check..nicuveo/rewrite-analysis). PR-URL: https://github.com/hasura/graphql-engine-mono/pull/3963 Co-authored-by: paritosh-08 <85472423+paritosh-08@users.noreply.github.com> GitOrigin-RevId: 5ec38e0e753f0c12096a350db0737658495e2f15
142 lines
3.4 KiB
YAML
142 lines
3.4 KiB
YAML
- description: Add a temp table
|
|
url: /v1/query
|
|
status: 200
|
|
query:
|
|
type: bulk
|
|
args:
|
|
- type: run_sql
|
|
args:
|
|
sql: |
|
|
create table temp_table(
|
|
col1 text,
|
|
col2 text,
|
|
id UUID NOT NULL DEFAULT gen_random_uuid()
|
|
);
|
|
|
|
- type: track_table
|
|
args:
|
|
schema: public
|
|
name: temp_table
|
|
|
|
- type: run_sql
|
|
args:
|
|
sql: |
|
|
insert into temp_table (col1, col2)
|
|
values
|
|
('temp1', 'temp2'),
|
|
('temp3', 'temp4'),
|
|
('temp5', 'temp6');
|
|
response:
|
|
- result_type: CommandOk
|
|
result:
|
|
- message: success
|
|
- result_type: CommandOk
|
|
result:
|
|
|
|
- description: Create a rest endpoint
|
|
url: /v1/query
|
|
status: 200
|
|
query:
|
|
type: bulk
|
|
args:
|
|
- type: create_query_collection
|
|
args:
|
|
name: temp_collection
|
|
definition:
|
|
queries:
|
|
- name: temp_query
|
|
query: query { temp_table { col1 col2 } }
|
|
- type: create_rest_endpoint
|
|
args:
|
|
url: temp_rest_API
|
|
name: temp_rest_API
|
|
methods:
|
|
- GET
|
|
definition:
|
|
query:
|
|
collection_name: temp_collection
|
|
query_name: temp_query
|
|
response:
|
|
- message: success
|
|
- message: success
|
|
|
|
- description: untrack the table
|
|
url: /v1/query
|
|
status: 500
|
|
query:
|
|
type: untrack_table
|
|
args:
|
|
table:
|
|
schema: public
|
|
name: temp_table
|
|
response:
|
|
internal:
|
|
- definition:
|
|
name: temp_query
|
|
query: query { temp_table { col1 col2 } }
|
|
reason: "Inconsistent object: In query collection \"temp_collection\" the query\
|
|
\ \"temp_query\" is invalid with the following error(s): \"$.temp_table: field\
|
|
\ 'temp_table' not found in object 'query_root'\". This query is being used\
|
|
\ in the following REST endpoint(s): \"temp_rest_API\""
|
|
name: query_collections temp_query in temp_collection
|
|
type: query_collections
|
|
path: $.args
|
|
error: cannot continue due to newly found inconsistent metadata
|
|
code: unexpected
|
|
|
|
- description: cleanup 1 (drop the endpoint)
|
|
url: /v1/query
|
|
status: 200
|
|
query:
|
|
type: drop_rest_endpoint
|
|
args:
|
|
name: temp_rest_API
|
|
response:
|
|
message: success
|
|
|
|
- description: untrack the table
|
|
url: /v1/query
|
|
status: 500
|
|
query:
|
|
type: untrack_table
|
|
args:
|
|
table:
|
|
schema: public
|
|
name: temp_table
|
|
response:
|
|
internal:
|
|
- definition:
|
|
name: temp_query
|
|
query: query { temp_table { col1 col2 } }
|
|
reason: "Inconsistent object: In query collection \"temp_collection\" the query\
|
|
\ \"temp_query\" is invalid with the following error(s): \"$.temp_table: field\
|
|
\ 'temp_table' not found in object 'query_root'\""
|
|
name: query_collections temp_query in temp_collection
|
|
type: query_collections
|
|
path: $.args
|
|
error: cannot continue due to newly found inconsistent metadata
|
|
code: unexpected
|
|
|
|
- description: cleanup 2 (drop the query collection)
|
|
url: /v1/query
|
|
status: 200
|
|
query:
|
|
type: drop_query_collection
|
|
args:
|
|
collection: temp_collection
|
|
cascade: true
|
|
response:
|
|
message: success
|
|
|
|
- description: cleanup 3 (drop the table)
|
|
url: /v1/query
|
|
status: 200
|
|
query:
|
|
type: run_sql
|
|
args:
|
|
sql: |
|
|
drop table temp_table
|
|
response:
|
|
result_type: CommandOk
|
|
result:
|