Skip to content

Support use first and last at same time as offset #563

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

Closed

Conversation

NateScarlet
Copy link

@NateScarlet NateScarlet commented Dec 18, 2018

Current implementation of max limit not correct when use first and last at same time
This operation is only valid when graphene_settings.RELAY_CONNECTION_MAX_LIMIT greater than 9999:

{
  users(first: 9999, last: 1) {
    edges {
      node {
        id
      }
    }
  }
}

Fixed it by using count = min(i for i in (first, last) if i)

@coveralls
Copy link

coveralls commented Dec 18, 2018

Coverage Status

Coverage decreased (-0.1%) to 94.376% when pulling b3bf937 on NateScarlet:connection-offset into f76f38e on graphql-python:master.

@zbyte64
Copy link
Collaborator

zbyte64 commented Mar 20, 2019

According to coveralls there are 3 new conditions not tested:

  • negative first
  • negative last
  • 0 max_limit

We should have tests for those.

@NateScarlet
Copy link
Author

According to coveralls there are 3 new conditions not tested:

* negative `first`

* negative `last`

* 0 `max_limit`

We should have tests for those.

Added

Copy link
Collaborator

@danpalmer danpalmer left a comment

Choose a reason for hiding this comment

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

Minor changes and clarifications would be great. Otherwise I think this looks good. Thanks for the contribution, it's great to see more strictness and good error messages.

@stale
Copy link

stale bot commented Jun 11, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jun 11, 2019
@stale stale bot removed the wontfix label Jun 12, 2019
@stale
Copy link

stale bot commented Aug 11, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Aug 11, 2019
@stale stale bot removed the wontfix label Aug 12, 2019
@NateScarlet
Copy link
Author

ping @danpalmer

@stale
Copy link

stale bot commented Oct 11, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Oct 11, 2019
@zbyte64 zbyte64 removed the wontfix label Oct 11, 2019
@stale
Copy link

stale bot commented Jan 16, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Jan 16, 2020
@jkimbo jkimbo removed the wontfix label Jan 20, 2020
@NateScarlet
Copy link
Author

I am closing this PR because my project has been rewritten in golang.

@thomas-arun
Copy link

@NateScarlet, Sorry to bring this thread back to life. But would you help me with the reason why you re wrote your project in Go Lang?
Did you use graphql-go ?

@NateScarlet
Copy link
Author

@NateScarlet, Sorry to bring this thread back to life. But would you help me with the reason why you re wrote your project in Go Lang? Did you use graphql-go ?

I switch to golang because it has better performance on same hardware.

I am using gqlgen

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.

7 participants