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

feat/rustberry integration v2 #3504

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
f8990c8
feat: initial rustberry integration
erikwrede Jul 23, 2023
16a2bb4
feat: initial rustberry integration
erikwrede Jul 23, 2023
d8f3d15
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 23, 2023
ca8129f
fix: add removed code back
erikwrede Jul 23, 2023
e7437e5
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 23, 2023
434653c
fix: pycharm removed imports
erikwrede Jul 23, 2023
7fd22d0
fix: parse
erikwrede Jul 23, 2023
d19a212
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 23, 2023
5b49773
chore: adjust workflow
erikwrede Jul 23, 2023
2558c2d
fix: correctly instantiate executor
erikwrede Jul 23, 2023
845aa38
fix: recreate lock
erikwrede Jul 23, 2023
aa2bb83
chore: fix parser
erikwrede Jul 23, 2023
377ac47
fix?: mark rustberry as non-optional
erikwrede Jul 23, 2023
d928e57
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 23, 2023
17097a3
chore: uncomment several tests
erikwrede Jul 23, 2023
20340ed
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jul 23, 2023
5f85b24
Merge branch 'main' into feat/rustberry-integration
erikwrede Jan 6, 2024
76aa2bd
Merge remote-tracking branch 'fork/feat/rustberry-integration' into f…
erikwrede May 18, 2024
a5beaf1
merge main into rustberry
erikwrede May 18, 2024
384d281
chore: update rustberry to v0.0.10
erikwrede May 18, 2024
59353ab
chore: update rustberry to v0.0.12
erikwrede May 18, 2024
4b6d378
chore: update rustberry to v0.0.12
erikwrede May 18, 2024
3ca28b3
test: use rustberry executor
erikwrede May 20, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 19 additions & 1 deletion poetry.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ libcst = {version = ">=0.4.7", optional = true}
rich = {version = ">=12.0.0", optional = true}
pyinstrument = {version = ">=4.0.0", optional = true}
graphlib_backport = {version = "*", python = "<3.9", optional = true}

rustberry = {version = ">=0.0.14", python=">=3.11"}
[tool.poetry.group.dev.dependencies]
asgiref = "^3.2"
ddtrace = ">=1.6.4"
Expand Down
78 changes: 38 additions & 40 deletions strawberry/schema/execute.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@
cast,
)

from graphql import GraphQLError, parse
from graphql import GraphQLError, parse, validate
from graphql import execute as original_execute
from graphql.validation import validate

from strawberry.exceptions import MissingQueryError
from strawberry.extensions.runner import SchemaExtensionsRunner
Expand All @@ -38,6 +37,7 @@

from strawberry.extensions import SchemaExtension
from strawberry.types import ExecutionContext
from strawberry.types.execution import Executor, ParseOptions
from strawberry.types.graphql import OperationType


Expand Down Expand Up @@ -82,6 +82,7 @@ async def execute(
execution_context: ExecutionContext,
execution_context_class: Optional[Type[GraphQLExecutionContext]] = None,
process_errors: Callable[[List[GraphQLError], Optional[ExecutionContext]], None],
executor: Executor,
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider making the executor parameter optional.

If the executor parameter is not provided, you could default to a standard executor like GraphQlCoreExecutor. This would make the function more flexible and backward-compatible.

Suggested change
executor: Executor,
executor: Executor = GraphQlCoreExecutor(),

) -> ExecutionResult:
extensions_runner = SchemaExtensionsRunner(
execution_context=execution_context,
Expand All @@ -95,30 +96,28 @@ async def execute(
if not execution_context.query:
raise MissingQueryError()

async with extensions_runner.parsing():
try:
if not execution_context.graphql_document:
execution_context.graphql_document = parse_document(
execution_context.query, **execution_context.parse_options
)
async with extensions_runner.parsing():
try:
if not execution_context.graphql_document:
executor.parse(execution_context)

except GraphQLError as exc:
execution_context.errors = [exc]
process_errors([exc], execution_context)
return ExecutionResult(
data=None,
errors=[exc],
extensions=await extensions_runner.get_extensions_results(),
)
except GraphQLError as exc:
execution_context.errors = [exc]
process_errors([exc], execution_context)
return ExecutionResult(
data=None,
errors=[exc],
extensions=await extensions_runner.get_extensions_results(),
)

if execution_context.operation_type not in allowed_operation_types:
raise InvalidOperationTypeError(execution_context.operation_type)

async with extensions_runner.validation():
_run_validation(execution_context)
if execution_context.errors:
process_errors(execution_context.errors, execution_context)
return ExecutionResult(data=None, errors=execution_context.errors)
async with extensions_runner.validation():
executor.validate(execution_context)
if execution_context.errors:
process_errors(execution_context.errors, execution_context)
return ExecutionResult(data=None, errors=execution_context.errors)

async with extensions_runner.executing():
if not execution_context.result:
Expand Down Expand Up @@ -180,6 +179,7 @@ def execute_sync(
execution_context: ExecutionContext,
execution_context_class: Optional[Type[GraphQLExecutionContext]] = None,
process_errors: Callable[[List[GraphQLError], Optional[ExecutionContext]], None],
executor: Executor,
) -> ExecutionResult:
extensions_runner = SchemaExtensionsRunner(
execution_context=execution_context,
Expand All @@ -193,30 +193,28 @@ def execute_sync(
if not execution_context.query:
raise MissingQueryError()

with extensions_runner.parsing():
try:
if not execution_context.graphql_document:
execution_context.graphql_document = parse_document(
execution_context.query, **execution_context.parse_options
)
with extensions_runner.parsing():
try:
if not execution_context.graphql_document:
executor.parse(execution_context)

except GraphQLError as exc:
execution_context.errors = [exc]
process_errors([exc], execution_context)
return ExecutionResult(
data=None,
errors=[exc],
extensions=extensions_runner.get_extensions_results_sync(),
)
except GraphQLError as exc:
execution_context.errors = [exc]
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (code-quality): Extract duplicate code into function (extract-duplicate-method)

process_errors([exc], execution_context)
return ExecutionResult(
data=None,
errors=[exc],
extensions=extensions_runner.get_extensions_results_sync(),
)

if execution_context.operation_type not in allowed_operation_types:
raise InvalidOperationTypeError(execution_context.operation_type)

with extensions_runner.validation():
_run_validation(execution_context)
if execution_context.errors:
process_errors(execution_context.errors, execution_context)
return ExecutionResult(data=None, errors=execution_context.errors)
with extensions_runner.validation():
executor.validate(execution_context)
if execution_context.errors:
process_errors(execution_context.errors, execution_context)
return ExecutionResult(data=None, errors=execution_context.errors)

with extensions_runner.executing():
if not execution_context.result:
Expand Down
58 changes: 58 additions & 0 deletions strawberry/schema/executors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
from typing import TYPE_CHECKING
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider adding a module-level docstring.

Adding a module-level docstring to strawberry/schema/executors.py would help other developers understand the purpose and usage of the module.

Suggested change
from typing import TYPE_CHECKING
"""
This module provides the necessary components for executing GraphQL queries
within the Strawberry framework. It includes error handling and type checking
utilities to ensure robust and type-safe query execution.
"""
from typing import TYPE_CHECKING
from graphql import GraphQLError


from graphql import GraphQLError, parse
from rustberry import QueryCompiler

from strawberry import Schema
from strawberry.types.execution import ExecutionContext, Executor

if TYPE_CHECKING:
from rustberry._rustberry import Document

RUSTBERRY_DOCUMENT_FIELD = "__rustberry_document"


class RustberryExecutor(Executor):
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider adding a docstring to the RustberryExecutor class.

Adding a docstring to the RustberryExecutor class would help other developers understand its purpose and usage.

Suggested change
class RustberryExecutor(Executor):
class RustberryExecutor(Executor):
"""
Executor for handling Rustberry-specific operations within the schema.
Args:
schema (Schema): The GraphQL schema to execute against.
"""

def __init__(self, schema: Schema) -> None:
super().__init__(schema)
self.compiler = QueryCompiler(schema.as_str())

def parse(self, execution_context: ExecutionContext) -> None:
document = self.compiler.parse(execution_context.query)
setattr(execution_context, RUSTBERRY_DOCUMENT_FIELD, document)
execution_context.graphql_document = self.compiler.gql_core_ast_mirror(document)

def validate(
self,
execution_context: ExecutionContext,
) -> None:
assert execution_context.graphql_document
document: Document = getattr(execution_context, RUSTBERRY_DOCUMENT_FIELD, None)
assert document, "Document not set - Required for Rustberry use"
validation_successful = self.compiler.validate(document)
if not validation_successful:
execution_context.errors = execution_context.errors or []
execution_context.errors.append(GraphQLError("Validation failed"))

Check warning on line 35 in strawberry/schema/executors.py

View check run for this annotation

Codecov / codecov/patch

strawberry/schema/executors.py#L34-L35

Added lines #L34 - L35 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider providing more context in the validation error message.

Providing more context in the validation error message, such as which part of the document failed validation, would help in debugging and understanding the issue.

Suggested change
execution_context.errors.append(GraphQLError("Validation failed"))
validation_errors = self.compiler.get_validation_errors(document)
error_message = f"Validation failed: {validation_errors}"
execution_context.errors.append(GraphQLError(error_message))



class RustberryExecutorV2(Executor):
def __init__(self, schema: Schema) -> None:
super().__init__(schema)
self.compiler = QueryCompiler(schema.as_str())

def parse(self, execution_context: ExecutionContext) -> None:
document = self.compiler.parse(execution_context.query)
setattr(execution_context, RUSTBERRY_DOCUMENT_FIELD, document)
execution_context.graphql_document = parse(execution_context.query)

def validate(
self,
execution_context: ExecutionContext,
) -> None:
assert execution_context.graphql_document
document: Document = getattr(execution_context, RUSTBERRY_DOCUMENT_FIELD, None)
assert document, "Document not set - Required for Rustberry use"
validation_successful = self.compiler.validate(document)
if not validation_successful:
execution_context.errors = execution_context.errors or []
execution_context.errors.append(GraphQLError("Validation failed"))

Check warning on line 58 in strawberry/schema/executors.py

View check run for this annotation

Codecov / codecov/patch

strawberry/schema/executors.py#L57-L58

Added lines #L57 - L58 were not covered by tests
9 changes: 9 additions & 0 deletions strawberry/schema/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
from strawberry.types.types import StrawberryObjectDefinition

from ..printer import print_schema
from ..types.execution import Executor, GraphQlCoreExecutor
from . import compat
from .base import BaseSchema
from .config import StrawberryConfig
Expand Down Expand Up @@ -82,6 +83,7 @@ def __init__(
Dict[object, Union[Type, ScalarWrapper, ScalarDefinition]]
] = None,
schema_directives: Iterable[object] = (),
executor_class: Optional[Type[Executor]] = None,
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider adding a docstring to the executor_class parameter.

Adding a docstring to the executor_class parameter would help other developers understand its purpose and usage.

Suggested change
executor_class: Optional[Type[Executor]] = None,
"""
executor_class: Optional[Type[Executor]] = None,
The class to use for executing queries. If not provided, a default executor will be used.
"""
executor_class: Optional[Type[Executor]] = None,

) -> None:
self.query = query
self.mutation = mutation
Expand Down Expand Up @@ -176,6 +178,11 @@ def __init__(
formatted_errors = "\n\n".join(f"❌ {error.message}" for error in errors)
raise ValueError(f"Invalid Schema. Errors:\n\n{formatted_errors}")

if executor_class:
self.executor = executor_class(self)
else:
self.executor = GraphQlCoreExecutor(self)

def get_extensions(
self, sync: bool = False
) -> List[Union[Type[SchemaExtension], SchemaExtension]]:
Expand Down Expand Up @@ -267,6 +274,7 @@ async def execute(
execution_context=execution_context,
allowed_operation_types=allowed_operation_types,
process_errors=self.process_errors,
executor=self.executor,
)

return result
Expand Down Expand Up @@ -299,6 +307,7 @@ def execute_sync(
execution_context=execution_context,
allowed_operation_types=allowed_operation_types,
process_errors=self.process_errors,
executor=self.executor,
)

return result
Expand Down
42 changes: 41 additions & 1 deletion strawberry/types/execution.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import abc
import dataclasses
from typing import (
TYPE_CHECKING,
Expand All @@ -12,7 +13,7 @@
)
from typing_extensions import TypedDict

from graphql import specified_rules
from graphql import parse, specified_rules, validate

from strawberry.utils.operation import get_first_operation, get_operation_type

Expand All @@ -29,6 +30,45 @@
from .graphql import OperationType


class Executor(abc.ABC):
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider adding a docstring to the Executor class.

Adding a docstring to the Executor class would help other developers understand its purpose and usage.

Suggested change
class Executor(abc.ABC):
class Executor(abc.ABC):
"""
Abstract base class for executing operations on a given schema.
Attributes:
schema (Schema): The GraphQL schema to execute operations on.
"""

def __init__(self, schema: Schema) -> None:
self.schema = schema

@abc.abstractmethod
def parse(self, execution_context: ExecutionContext) -> None: ...

@abc.abstractmethod
def validate(
self,
execution_context: ExecutionContext,
) -> None: ...


class GraphQlCoreExecutor(Executor):
Copy link
Contributor

Choose a reason for hiding this comment

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

suggestion: Consider adding a docstring to the GraphQlCoreExecutor class.

Adding a docstring to the GraphQlCoreExecutor class would help other developers understand its purpose and usage.

Suggested change
class GraphQlCoreExecutor(Executor):
class GraphQlCoreExecutor(Executor):
"""
Executes GraphQL queries using the provided schema.
This class extends the Executor to handle the execution of GraphQL
queries, mutations, and subscriptions.
"""

def __init__(self, schema: Schema) -> None:
super().__init__(schema)

def parse(self, execution_context: ExecutionContext) -> None:
execution_context.graphql_document = parse(
execution_context.query, **execution_context.parse_options
)

def validate(
self,
execution_context: ExecutionContext,
) -> None:
if (
len(execution_context.validation_rules) > 0
and execution_context.errors is None
):
assert execution_context.graphql_document
execution_context.errors = validate(
execution_context.schema._schema,
execution_context.graphql_document,
execution_context.validation_rules,
)


@dataclasses.dataclass
class ExecutionContext:
query: Optional[str]
Expand Down
10 changes: 8 additions & 2 deletions tests/benchmarks/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import strawberry
from strawberry.directive import DirectiveLocation
from strawberry.schema.executors import RustberryExecutor, RustberryExecutorV2


@strawberry.type
Expand Down Expand Up @@ -79,7 +80,12 @@ def uppercase(value: str) -> str:
return value.upper()


schema = strawberry.Schema(query=Query, subscription=Subscription)
schema = strawberry.Schema(
query=Query, subscription=Subscription, executor_class=RustberryExecutor
)
schema_with_directives = strawberry.Schema(
query=Query, directives=[uppercase], subscription=Subscription
query=Query,
directives=[uppercase],
subscription=Subscription,
executor_class=RustberryExecutorV2,
)
3 changes: 2 additions & 1 deletion tests/benchmarks/schema.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
from typing_extensions import Annotated

import strawberry
from strawberry.schema.executors import RustberryExecutor


@strawberry.enum
Expand Down Expand Up @@ -178,4 +179,4 @@ async def search(
]


schema = strawberry.Schema(query=Query)
schema = strawberry.Schema(query=Query, executor_class=RustberryExecutor)
Loading
Loading