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(relay): Allow to customize max_results per connection in relay #3746

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bellini666
Copy link
Member

@bellini666 bellini666 commented Jan 4, 2025

Fix #3734

Summary by Sourcery

New Features:

  • Added max_results parameter to the @connection decorator to customize the maximum number of results per connection field, overriding the global schema setting.

@bellini666 bellini666 self-assigned this Jan 4, 2025
Copy link
Contributor

sourcery-ai bot commented Jan 4, 2025

Reviewer's Guide by Sourcery

This pull request introduces a new feature that allows developers to customize the maximum number of results returned per connection in a Relay schema. This is achieved by adding a max_results parameter to the @strawberry.relay.connection decorator. The default maximum number of results is defined in the schema configuration and defaults to 100.

Sequence diagram for relay connection resolution with max_results

sequenceDiagram
    participant Client
    participant Connection
    participant ConnectionExtension
    participant ListConnection

    Client->>Connection: Query with connection field
    Connection->>ConnectionExtension: resolve()
    Note over ConnectionExtension: Check max_results parameter
    ConnectionExtension->>ListConnection: resolve_connection(max_results)
    Note over ListConnection: Apply max_results limit
    ListConnection-->>ConnectionExtension: Limited results
    ConnectionExtension-->>Connection: Connection with paginated data
    Connection-->>Client: Final response
Loading

Class diagram showing the updated ConnectionExtension and connection field changes

classDiagram
    class ConnectionExtension {
        +connection_type: type[Connection[Node]]
        +max_results: Optional[int]
        +__init__(max_results: Optional[int])
        +apply(field: StrawberryField)
        +resolve(...)
        +resolve_async(...)
    }
    class connection {
        <<function>>
        +__call__(..., max_results: Optional[int])
    }
    note for ConnectionExtension "New max_results parameter
to override default limit"
Loading

File-Level Changes

Change Details Files
Added max_results argument to the connection decorator and updated the resolver to respect this new argument.
  • Added the max_results parameter to the @connection decorator.
  • Modified the resolve_connection functions to accept and use the max_results parameter.
  • Updated the documentation to reflect the new max_results parameter.
  • Added tests to verify the functionality of the max_results parameter.
  • Updated the Slice utility function to prioritize the max_results parameter passed to the resolver over the schema's default value if provided.
strawberry/relay/fields.py
strawberry/relay/types.py
strawberry/relay/utils.py
tests/relay/test_connection.py
docs/guides/relay.md
Added a release note for the new feature.
  • Documented the new feature in the release notes, explaining its purpose and usage with examples.
RELEASE.md

Assessment against linked issues

Issue Objective Addressed Explanation
#3734 Allow customization of max_results per connection in relay
#3734 Provide a way to specify max_results in the connection decorator
#3734 Ensure the customized max_results can override the schema-level configuration

Possibly linked issues


Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@bellini666 bellini666 requested a review from patrick91 January 4, 2025 11:51
Copy link
Contributor

@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 @bellini666 - I've reviewed your changes - here's some feedback:

Overall Comments:

  • There's a typo in the documentation examples where 'strawberry' is misspelled as 'strawerry'
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟡 Testing: 2 issues found
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: 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 and I'll use the feedback to improve your reviews.

tests/relay/test_connection.py Show resolved Hide resolved
tests/relay/test_connection.py Show resolved Hide resolved
class Query:
fruits: ListConnection[Fruit] = relay.connection(max_results=10_000)
```

### Custom connection pagination

The default `relay.Connection` class don't implement any pagination logic, and
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (typo): Typo: "don't" should be "doesn't"

The sentence should read "The default relay.Connection class doesn't implement any pagination logic..."

@botberry
Copy link
Member

botberry commented Jan 4, 2025

Thanks for adding the RELEASE.md file!

Here's a preview of the changelog:


Add the ability to override the "max results" a relay's connection can return on
a per-field basis.

The default value for this is defined in the schema's config, and set to 100
unless modified by the user. Now, that per-field value will take precedence over
it.

For example:

@strawerry.type
class Query:
    # This will still use the default value in the schema's config
    fruits: ListConnection[Fruit] = relay.connection()

    # This will reduce the maximum number of results to 10
    limited_fruits: ListConnection[Fruit] = relay.connection(max_results=10)

    # This will increase the maximum number of results to 10
    higher_limited_fruits: ListConnection[Fruit] = relay.connection(max_results=10_000)

Note that this only affects ListConnection and subclasses. If you are
implementing your own connection resolver, there's an extra keyword named
max_results: int | None that will be passed to it.

Here's the tweet text:

🆕 Release (next) is out! Thanks to @_bellini666 for the PR 👏

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

Copy link

codecov bot commented Jan 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.28%. Comparing base (6bc7332) to head (ec77ac9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3746   +/-   ##
=======================================
  Coverage   97.28%   97.28%           
=======================================
  Files         502      502           
  Lines       33375    33395   +20     
  Branches     5477     5482    +5     
=======================================
+ Hits        32469    32489   +20     
  Misses        697      697           
  Partials      209      209           

Copy link

codspeed-hq bot commented Jan 4, 2025

CodSpeed Performance Report

Merging #3746 will not alter performance

Comparing relay_max_results_per_field (ec77ac9) with main (6bc7332)

Summary

✅ 21 untouched benchmarks

Comment on lines +21 to +22
# This will increase the maximum number of results to 10
higher_limited_fruits: ListConnection[Fruit] = relay.connection(max_results=10_000)
Copy link
Member

Choose a reason for hiding this comment

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

This is a typo, right?

Suggested change
# This will increase the maximum number of results to 10
higher_limited_fruits: ListConnection[Fruit] = relay.connection(max_results=10_000)
# This will increase the maximum number of results to 10000
higher_limited_fruits: ListConnection[Fruit] = relay.connection(max_results=10_000)

Copy link
Member

@DoctorJohn DoctorJohn left a comment

Choose a reason for hiding this comment

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

LGTM! Straightforward implementation and well-tested :)

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.

Allow to customise max_results per connection in relay
3 participants