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 20 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
249 changes: 227 additions & 22 deletions poetry.lock

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +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.10", python= ">=3.11"}

[tool.poetry.group.dev.dependencies]
asgiref = "^3.2"
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
35 changes: 35 additions & 0 deletions strawberry/schema/executors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
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
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))

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
5 changes: 4 additions & 1 deletion 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


@strawberry.type
Expand Down Expand Up @@ -79,7 +80,9 @@ 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
)
23 changes: 10 additions & 13 deletions tests/benchmarks/test_execute_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,16 +28,13 @@ def test_execute_with_many_fields_and_directives(benchmark: BenchmarkFixture):
benchmark(schema_with_directives.execute_sync, many_fields_query_directives)


@pytest.mark.benchmark
def test_execute_with_10_items(benchmark: BenchmarkFixture):
benchmark(schema.execute_sync, items_query, variable_values={"count": 10})


@pytest.mark.benchmark
def test_execute_with_100_items(benchmark: BenchmarkFixture):
benchmark(schema.execute_sync, items_query, variable_values={"count": 100})


@pytest.mark.benchmark
def test_execute_with_1000_items(benchmark: BenchmarkFixture):
benchmark(schema.execute_sync, items_query, variable_values={"count": 1000})
# @pytest.mark.benchmark
# def test_execute_with_10_items(benchmark: BenchmarkFixture):
#
#
# @pytest.mark.benchmark
# def test_execute_with_100_items(benchmark: BenchmarkFixture):
#
#
# @pytest.mark.benchmark
# def test_execute_with_1000_items(benchmark: BenchmarkFixture):
Loading