Skip to content
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 support for using FastAPI APIRouter arguments in GraphQLRouter #3442

Merged
merged 5 commits into from Apr 14, 2024

Conversation

nparamonov
Copy link
Member

@nparamonov nparamonov commented Apr 6, 2024

Description

Add the ability to pass additional arguments to the APIRouter via kwargs for flexibility.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @nparamonov - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟡 Testing: 3 issues found
  • 🟢 Complexity: all looks good
  • 🟢 Docstrings: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

strawberry/fastapi/router.py Show resolved Hide resolved
tests/fastapi/test_openapi.py Show resolved Hide resolved
tests/fastapi/test_openapi.py Show resolved Hide resolved
tests/fastapi/test_openapi.py Show resolved Hide resolved
@botberry
Copy link
Member

botberry commented Apr 6, 2024

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


This release adds support for using FastAPI APIRouter arguments in GraphQLRouter.

Now you have the opportunity to specify parameters such as tags, route_class,
deprecated, include_in_schema, etc:

import strawberry

from fastapi import FastAPI
from strawberry.fastapi import GraphQLRouter


@strawberry.type
class Query:
    @strawberry.field
    def hello(self) -> str:
        return "Hello World"


schema = strawberry.Schema(Query)

graphql_app = GraphQLRouter(schema, tags=["graphql"])

app = FastAPI()
app.include_router(graphql_app, prefix="/graphql")

Here's the tweet text:

🆕 Release (next) is out! Thanks to Nikita Paramonov for the PR 👏

Get it here 👉 https://strawberry.rocks/release/(next)

Copy link

codecov bot commented Apr 7, 2024

Codecov Report

Merging #3442 (52f609c) into main (04e5d4e) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3442   +/-   ##
=======================================
  Coverage   96.48%   96.48%           
=======================================
  Files         509      509           
  Lines       32705    32717   +12     
  Branches     5404     5404           
=======================================
+ Hits        31554    31566   +12     
  Misses        940      940           
  Partials      211      211           

Copy link

codspeed-hq bot commented Apr 7, 2024

CodSpeed Performance Report

Merging #3442 will degrade performances by 29.07%

Comparing nparamonov:fastapi-kwargs (52f609c) with main (04e5d4e)

Summary

❌ 1 regressions
✅ 12 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main nparamonov:fastapi-kwargs Change
test_execute_basic 9.8 ms 13.9 ms -29.07%

Copy link
Member

@patrick91 patrick91 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! Thanks 😊

@patrick91 patrick91 merged commit 7a6201c into strawberry-graphql:main Apr 14, 2024
63 of 64 checks passed
@botberry
Copy link
Member

Thanks for contributing to Strawberry! 🎉 You've been invited to join
the Strawberry GraphQL organisation 😊

You can also request a free sticker by filling this form: https://forms.gle/dmnfQUPoY5gZbVT67

And don't forget to join our discord server: https://strawberry.rocks/discord 🔥

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants