-
Notifications
You must be signed in to change notification settings - Fork 3
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
Add GraphQL transport #71
Conversation
ee08dd5
to
8d61dde
Compare
I have a had a quick look, but I am going to ignore this until we iterate on and merge some of the other recent PRs first, as you suggest. |
Lets do that. When we are done iterating, we can enjoy some of the graph recursion xD |
8d61dde
to
31f82e6
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #71 +/- ##
==========================================
+ Coverage 87.76% 88.06% +0.30%
==========================================
Files 28 31 +3
Lines 1193 1307 +114
==========================================
+ Hits 1047 1151 +104
- Misses 146 156 +10 ☔ View full report in Codecov by Sentry. |
Apparently camelCase is the convention for field names https://www.apollographql.com/docs/graphos/schema-design/guides/naming-conventions#casing so i have adjusted accordingly |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
First pass comments
I will rebase this on monday |
f6fd6a4
to
ef2c5c4
Compare
@GDYendell Rebased. Also did the rename of create_type as discussed this morning |
Apply upstream feedback Remove fastapi layer from gql Process review comments Rebase create_type -> create_strawberry_type
24d8e22
to
ef5c8e3
Compare
Hmm - the tests passed now that its in my branch... LGTM I guess |
Ah I guess the rebase might have solved it |
Note that puts still do not work here because of the problem where the tasks are added to the wrong event loop. So we either need to add something like def _create_sender_callback(self, attribute, controller):
async def callback(value):
future = asyncio.run_coroutine_threadsafe(
attribute.sender.put(controller, attribute, value), self._loop
)
await asyncio.wrap_future(future)
return callback or we need to figure out #98. I think we should just merge this for now, though. |
The puts issue is across transports so I agree we should merge this and look towards #98 which I believe solves the problem |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An additional "backend" for FastCS which provides a GraphQL api.
The API is fully discoverable and demoable from the autogenerated "GraphiQL gui" at "{domain}/graphql".
I choose to use the FastAPI "Strawberry" integration as it is the recommended library by FastAPI.
The following considersations have been taken to conform with best practices:
Example:
Example:
Example:
Further: