remote relationship create api #2850

tirumaraiselvan · 2019-09-09T13:31:56Z

Description

Independent PR for Remote Joins (excludes execution)

Affected components

Server

Todo

netlify · 2019-09-09T13:36:38Z

Deploy preview for hasura-docs ready!

Built with commit c933b35

https://deploy-preview-2850--hasura-docs.netlify.com

server/src-lib/Hasura/RQL/Types/RemoteRelationship.hs

hasura-bot · 2019-09-17T14:03:58Z

Review app for commit e7a23cd deployed to Heroku: https://hge-ci-pull-2850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2850-e7a23cd9

jberryman · 2019-09-18T01:33:20Z

This seems fine, but it would be wonderful if we could get documentation on all exported functions and data types.

Is there anything specifically you think could use more review/attention?

tirumaraiselvan · 2019-09-18T02:24:14Z

This seems fine, but it would be wonderful if we could get documentation on all exported functions and data types.

Is there anything specifically you think could use more review/attention?

remote relationship create api #2850 (comment)
Can we do some other general cleanup to reduce nesting etc?

2) Run a GraphQL server per test thread 3) GraphQL proxy which sets a prefix to the types and top-level fields of the schema, and thus can be added a remote.

hasura-bot · 2019-09-23T12:48:17Z

Review app for commit 1ddd4f2 deployed to Heroku: https://hge-ci-pull-2850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2850-1ddd4f23

hasura-bot · 2019-09-24T10:00:07Z

Review app for commit dc35557 deployed to Heroku: https://hge-ci-pull-2850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2850-dc355577

hasura-bot · 2019-09-24T10:59:47Z

Review app for commit 6343d00 deployed to Heroku: https://hge-ci-pull-2850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2850-6343d00a

Tests for creation of remote relationships (close #60)

tirumaraiselvan · 2019-09-24T11:40:26Z

server/graphql-engine.cabal

@@ -61,6 +61,7 @@ library
                     , wai
                     , postgresql-binary
                     , process
+                     , validation


monad-validate might remove some boilerplate

I tried doing this and TBH it didn't make much of a difference. So just leaving this as is atm.

hasura-bot · 2019-09-24T12:03:49Z

Review app for commit 6443125 deployed to Heroku: https://hge-ci-pull-2850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2850-64431252

jberryman · 2019-09-24T13:52:16Z

@nizar-m Would you mind giving a quick summary of how the new test framework works, and what it can do?

Also is there any part in particular that you'd like closer review on?

nizar-m · 2019-09-25T00:14:43Z

@nizar-m Would you mind giving a quick summary of how the new test framework works, and what it can do?

Also is there any part in particular that you'd like closer review on?

1) Use Hasura GraphQL Engine as remote
What we were initially planning to do was running another Hasura GraphQL engine as a remote GraphQL server and attach that as remote. This way we can test with a very rich remote GraphQL schema with much lesser amount of configuration. So now, before any test, we will have to setup the remote Hasura GraphQL engine, which would be an extra configuration setup.

Then it evolved to why not create a proxy to GraphQL server which will add prefixes to the top-level fields of operations (query, mutation, subscription), and to the names of all object, input object and enum types. Then add this proxy as remote itself. That way we do not have to create another remote Hasura GraphQL engine, and so the current test configuration would work.

The implementation of the prefixer proxy is in tests-py/gql_prefixer_proxy.py. In tests-py/graphql_server.py this proxy is linked to the path /graphql-prefixer-proxy. So now as remote GraphQL servers, we have those that are implemented in Graphene, plus this proxy.

1.1) Cascade effect 1: Separate remote GraphQL servers for each test thread
The GraphQL remote server is dependent on GraphQL engine now, which is different for the different parallel test threads. So separate remote GrapQL servers are required for each test thread, which should run on different ports. So we should generate a list of ports equal to the number of parallel threads and assign these ports to the remote servers of each thread (in conftest.py ).

Fixture remote_gql_server handles the setup of the remote graphql server. Factory fixture remote_get_url provides the full URL when the path to the remote schema is provided.

1.2) Cascade effect 2: Replace localhost:5000 with ${REMOTE_GRAPHQL_ROOT_URL}
The port of remote_gql_server in tests files cannot be the constant 5000 anymore. Any test file which uses the remote GraphQL server has to be templated now. The current implementation is as follows

The remote_gql_fixture sets the environmental variable REMOTE_GRAPHQL_ROOT_URL when the graphql server is running (and resets it when the server stops running).
Add resolver and constructor to yaml (yaml_utils.py) so that anything which looks like ${ENV_NAME} in yaml file will be replaced by ENV_VALUE

nizar-m · 2019-09-25T01:49:19Z

The last yaml templating part is something that should have a second look. The solution for now works, but it may be confusing for people who are writing tests for the first time. We may move the whole tests into the code, or we may use templating languages like jsonnet or dhall or ytt.

If we are using something like jsonnet, ytt or dhall, we need to look at whether they support insert ordering for objects.

Looks jsonnet and dhall would not support insert order for jsons for now. ytt on the other hand seems to support it, at least on their playground, but it lacks python bindings.

- Run pytest using python 3.7

hasura-bot · 2019-09-25T10:29:08Z

Review app for commit ba8f465 deployed to Heroku: https://hge-ci-pull-2850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2850-ba8f4658

nizar-m · 2019-09-25T14:30:34Z

server/tests-py/gql_prefixer_proxy.py

+   def get(self, request):
+      return Response(HTTPStatus.METHOD_NOT_ALLOWED)
+
+   def _assert_prefixes(self, introspect):


Here we are ensuring that gql proxy is adding prefixes as intended.

nizar-m · 2019-09-25T14:34:18Z

server/tests-py/graphql_server.py

@@ -308,7 +313,7 @@ def post(self, req):
        if not req.json:
            return Response(HTTPStatus.BAD_REQUEST)
        res = character_interface_schema.execute(req.json['query'])
-        respDict = res.to_dict()
+        respDict = to_dict(res)


These changes exist because we upgraded to Graphene-3.
Some of the tests which we could not run because graphql-core was throwing error can be done now.
Also the errors are much more descriptive with Graphene 3, which includes stuff like the field name and the location of the field in the query.

nizar-m · 2019-09-25T14:36:55Z

server/tests-py/queries/remote_schemas/add_remote_schema_err_missing_arg.yaml

@@ -11,6 +11,6 @@ query:
  args:
    name: err-missing-arg
    definition:
-      url: http://localhost:5000/iface-graphql-err-missing-arg
+      url: ${REMOTE_GRAPHQL_ROOT_URL}/iface-graphql-err-missing-arg


Take the root url of remote GraphQL server from the environmental variable ${REMOTE_GRAPHQL_ROOT_URL} instead. This will be set by the remote_gql_server fixture when the remote server is running.

Some deferred websocket tests can be run now

…elvan/graphql-engine into issue-63

Run pytest with python 3.7 (close #63)

hasura-bot · 2019-09-26T07:37:23Z

Review app for commit 9433aa5 deployed to Heroku: https://hge-ci-pull-2850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2850-9433aa53

tirumaraiselvan · 2019-09-26T21:43:16Z

server/src-lib/Hasura/RQL/DDL/RemoteRelationship/Validate.hs

+  let initFieldCalls = NE.init $ rrRemoteFields remoteRel
+      leafFieldCall = NE.last $ rrRemoteFields remoteRel
+  (leafParentTypeInfo, leafParentTypeMap) <-
+    foldl


use foldl' (see: #2933 (comment))

hasura-bot · 2019-10-08T09:59:20Z

Review app for commit 2d9b323 deployed to Heroku: https://hge-ci-pull-2850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2850-2d9b3233

hasura-bot · 2019-10-09T11:34:18Z

Review app for commit c933b35 deployed to Heroku: https://hge-ci-pull-2850.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2850-c933b351

hasura-bot · 2020-04-24T07:37:49Z

Review app https://hge-ci-pull-2850.herokuapp.com is deleted

tirumaraiselvan assigned jberryman Sep 9, 2019

tirumaraiselvan requested review from jberryman and nizar-m September 9, 2019 13:32

wip remote relationship create api

e7a23cd

tirumaraiselvan force-pushed the add-remote-relationship-no-op branch from 9676a16 to e7a23cd Compare September 17, 2019 13:49

tirumaraiselvan changed the title ~~wip remote relationship create api~~ remote relationship create api Sep 17, 2019

tirumaraiselvan commented Sep 17, 2019

View reviewed changes

server/src-lib/Hasura/RQL/Types/RemoteRelationship.hs Show resolved Hide resolved

wawhal mentioned this pull request Sep 18, 2019

Remote Joins: Create relationships across database and remote schemas #2395

Closed

tirumaraiselvan mentioned this pull request Sep 18, 2019

Remote Joins: Create relationships across database and remote schemas #2392

Merged

nizar-m and others added 4 commits September 18, 2019 20:34

1) Tests for creation of remote relationships

c750a21

2) Run a GraphQL server per test thread 3) GraphQL proxy which sets a prefix to the types and top-level fields of the schema, and thus can be added a remote.

Fix errors on remote query timeout test

fe1a33f

Test creation with multi-nested field

b6b0b2f

allow multiple list elements in join arguements

1ddd4f2

Remove xfail for tests with array args

070f5a2

tirumaraiselvan force-pushed the add-remote-relationship-no-op branch from 21b9153 to dc35557 Compare September 24, 2019 09:52

tirumaraiselvan force-pushed the add-remote-relationship-no-op branch from dc35557 to 6343d00 Compare September 24, 2019 10:34

refactor

88427cb

tirumaraiselvan force-pushed the add-remote-relationship-no-op branch from 6343d00 to 88427cb Compare September 24, 2019 11:35

Merge pull request #61 from tirumaraiselvan/issue-60

6443125

Tests for creation of remote relationships (close #60)

tirumaraiselvan commented Sep 24, 2019

View reviewed changes

jberryman previously approved these changes Sep 24, 2019

View reviewed changes

no-op

ba8f465

tirumaraiselvan dismissed jberryman’s stale review via ba8f465 September 25, 2019 10:20

- Add python 3.7 to server-builder docker image

20de1a6

- Run pytest using python 3.7

fix the fixture for remote schema timeout tests

2a8baeb

nizar-m reviewed Sep 25, 2019

View reviewed changes

nizar-m and others added 4 commits September 26, 2019 11:57

Graphene 3 supports null value for input arguments.

f3687f1

Some deferred websocket tests can be run now

Merge branch 'add-remote-relationship-no-op' of github.com:tirumarais…

da92ef2

…elvan/graphql-engine into issue-63

Bring back ciignore

1edd8f7

Merge pull request #65 from tirumaraiselvan/issue-63

9433aa5

Run pytest with python 3.7 (close #63)

tirumaraiselvan commented Sep 26, 2019

View reviewed changes

remove hasura_fields from API

2d9b323

create functions

c933b35

tirumaraiselvan closed this Apr 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

remote relationship create api #2850

remote relationship create api #2850

tirumaraiselvan commented Sep 9, 2019 •

edited

Loading

netlify bot commented Sep 9, 2019 •

edited

Loading

hasura-bot commented Sep 17, 2019

jberryman commented Sep 18, 2019

tirumaraiselvan commented Sep 18, 2019

hasura-bot commented Sep 23, 2019

hasura-bot commented Sep 24, 2019

hasura-bot commented Sep 24, 2019

tirumaraiselvan Sep 24, 2019

tirumaraiselvan Sep 25, 2019

hasura-bot commented Sep 24, 2019

jberryman commented Sep 24, 2019

nizar-m commented Sep 25, 2019 •

edited

Loading

nizar-m commented Sep 25, 2019

hasura-bot commented Sep 25, 2019

nizar-m Sep 25, 2019

nizar-m Sep 25, 2019

nizar-m Sep 25, 2019

hasura-bot commented Sep 26, 2019

tirumaraiselvan Sep 26, 2019

hasura-bot commented Oct 8, 2019

hasura-bot commented Oct 9, 2019

hasura-bot commented Apr 24, 2020

remote relationship create api #2850

remote relationship create api #2850

Conversation

tirumaraiselvan commented Sep 9, 2019 • edited Loading

Description

Affected components

Todo

netlify bot commented Sep 9, 2019 • edited Loading

hasura-bot commented Sep 17, 2019

jberryman commented Sep 18, 2019

tirumaraiselvan commented Sep 18, 2019

hasura-bot commented Sep 23, 2019

hasura-bot commented Sep 24, 2019

hasura-bot commented Sep 24, 2019

tirumaraiselvan Sep 24, 2019

Choose a reason for hiding this comment

tirumaraiselvan Sep 25, 2019

Choose a reason for hiding this comment

hasura-bot commented Sep 24, 2019

jberryman commented Sep 24, 2019

nizar-m commented Sep 25, 2019 • edited Loading

nizar-m commented Sep 25, 2019

hasura-bot commented Sep 25, 2019

nizar-m Sep 25, 2019

Choose a reason for hiding this comment

nizar-m Sep 25, 2019

Choose a reason for hiding this comment

nizar-m Sep 25, 2019

Choose a reason for hiding this comment

hasura-bot commented Sep 26, 2019

tirumaraiselvan Sep 26, 2019

Choose a reason for hiding this comment

hasura-bot commented Oct 8, 2019

hasura-bot commented Oct 9, 2019

hasura-bot commented Apr 24, 2020

tirumaraiselvan commented Sep 9, 2019 •

edited

Loading

netlify bot commented Sep 9, 2019 •

edited

Loading

nizar-m commented Sep 25, 2019 •

edited

Loading