Page MenuHomeSoftware Heritage

Add error handling for pagination input arguments
ClosedPublic

Authored by jayeshv on Jun 14 2022, 10:20 AM.

Details

Summary

Logic to validate first and after arguments in pagination requests.
PaginationError will be raised on an invalid argument.

Related to T4261

Diff Detail

Repository
rDGQL GraphQL API
Branch
pagination-errors
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 29850
Build 46656: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 46655: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D7984 (id=28766)

Rebasing onto f32d98cd15...

Current branch diff-target is up to date.
Changes applied before test
commit 090ce1e2f4ebcc9097a76f380727fb945de2be65
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 13 18:03:21 2022 +0200

    Add error handling for pagination input arguments
    
    Logic to validate first and after arguments in pagination requests.
    PaginationError will be raised on an invalid argument.
    
    Related to T4261

See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/53/ for more details.

Build is green

Patch application report for D7984 (id=28767)

Rebasing onto f32d98cd15...

Current branch diff-target is up to date.
Changes applied before test
commit e75268003ad58a3803d00a5fe8a924402abae076
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 13 18:03:21 2022 +0200

    Add error handling for pagination input arguments
    
    Logic to validate first and after arguments in pagination requests.
    PaginationError will be raised on an invalid argument.
    
    Related to T4261

See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/54/ for more details.

anlambert added a subscriber: anlambert.

Looks good to me. I added some nitpick comments to handle.

swh/graphql/resolvers/base_connection.py
122

Using except Exception as e: is sufficient here as all caught exceptions derive from it.

133

You could indicate the max page size in error message here.

This revision is now accepted and ready to land.Jun 14 2022, 10:51 AM

Build is green

Patch application report for D7984 (id=28769)

Rebasing onto f32d98cd15...

Current branch diff-target is up to date.
Changes applied before test
commit 67f1ae16f549d6a5d11be6af4f8102c352060482
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Mon Jun 13 18:03:21 2022 +0200

    Add error handling for pagination input arguments
    
    Logic to validate first and after arguments in pagination requests.
    PaginationError will be raised on an invalid argument.
    
    Related to T4261

See https://jenkins.softwareheritage.org/job/DGQL/job/tests-on-diff/55/ for more details.

jayeshv added inline comments.
swh/graphql/resolvers/base_connection.py
122

I added those to be explicit about possible exceptions. This will also let us handle different errors and I think is a preferred practice.
Bare exception is caught to be safe :)

swh/graphql/resolvers/base_connection.py
122

Honestly I do not see the point to proceed like this if you have the same error handling for all caught exceptions but whatever.

swh/graphql/resolvers/base_connection.py
122

Sorry, I removed the base exception in another diff D7986