Paths

Table of Contentst

Differential D7984

Add error handling for pagination input arguments
ClosedPublic
Actions

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

Details

Reviewers

anlambert

Group Reviewers

Reviewers

Maniphest Tasks

T4261: Swh-graphql Code cleanup

Commits

rDGQL67f1ae16f549: Add error handling for pagination input arguments

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 jenkins	Jenkins console · Jenkins
Build 46655: arc lint + arc unit

Event Timeline

jayeshv created this revision.Jun 14 2022, 10:20 AM

Herald added a reviewer: Reviewers. · View Herald TranscriptJun 14 2022, 10:20 AM

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.

Harbormaster completed remote builds in B29849: Diff 28766.Jun 14 2022, 10:22 AM

jayeshv requested review of this revision.Jun 14 2022, 10:22 AM

snapshot branch updates

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.

Harbormaster completed remote builds in B29850: Diff 28767.Jun 14 2022, 10:41 AM

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

review comments

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.

Harbormaster completed remote builds in B29852: Diff 28769.Jun 14 2022, 11:21 AM

jayeshv marked an inline comment as done.Jun 14 2022, 11:22 AM

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 :)