Page MenuHomeSoftware Heritage

Add filters for latest visit status
ClosedPublic

Authored by jayeshv on Sep 21 2022, 12:07 PM.

Details

Summary

Possibility to filter a latest vist status by status state and
require snapshot.

Diff Detail

Repository
rDGQL GraphQL API
Branch
visit-status-filter
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 31650
Build 49508: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 49507: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D8515 (id=30663)

Rebasing onto ebc6e44534...

Current branch diff-target is up to date.
Changes applied before test
commit 01343d4509bda4d30141078db5287c03e52e58fb
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Sep 21 11:36:00 2022 +0200

    Add filters for latest visit status
    
    Possibility to filter a latest vist status by status state and
    require snapshot.

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

anlambert added a subscriber: anlambert.
anlambert added inline comments.
swh/graphql/resolvers/resolvers.py
82–88

Why the use of that NullableObjectError exception ? The call to resolver should already return None if the visit status search does not return any result, no ?

swh/graphql/schema/schema.graphql
200

Some statuses are misssing, full list is: "created", "ongoing", "full", "partial", "not_found", "failed"

200

s/VistStatusState/VisitStatusState/

This revision now requires changes to proceed.Sep 21 2022, 1:53 PM

Build is green

Patch application report for D8515 (id=30665)

Rebasing onto ebc6e44534...

Current branch diff-target is up to date.
Changes applied before test
commit e7c1ca63bfd063d4747f7e0ed96743dfd6d5698a
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Sep 21 11:36:00 2022 +0200

    Add filters for latest visit status
    
    Possibility to filter a latest vist status by status state and
    require snapshot.

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

jayeshv added inline comments.
swh/graphql/resolvers/resolvers.py
82–88

resolver(obj, info, **kw) will always return an object, not None. This will create issues for objects with mandatory sub fields.
that was the issue with the empty snapshot issue. This is a generic fix that handles this case in every resolver.
I have another diff on top of this change for that.

swh/graphql/resolvers/resolvers.py
82–88

Ack, I still think that it is simpler if a resolver could return None as some nodes can be nullable in the schema but whatever.

swh/graphql/tests/functional/test_visit_node.py
128–154

Could you ask for the snapshot in the query and test it is not null in the result ? This will make your test more consistent.

jayeshv added inline comments.
swh/graphql/resolvers/resolvers.py
82–88

To add some more context.
There are two types of behavior while getting a node.
1: should show an object not found error to the client. That is the default behavior. (eg: get origin by URL)

2: No error but return None (eg latestStatus in visit, latestVisit in origin, directory in revision etc) the resolver should fail silently.

I will raise this exception in a few more places in another diff.

Build is green

Patch application report for D8515 (id=30671)

Rebasing onto d445ce72ba...

First, rewinding head to replay your work on top of it...
Applying: Add filters for latest visit status
Changes applied before test
commit fb22432d5dee40102ad08268640b43b896f23ecb
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Sep 21 11:36:00 2022 +0200

    Add filters for latest visit status
    
    Possibility to filter a latest vist status by status state and
    require snapshot.

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

This revision is now accepted and ready to land.Sep 21 2022, 3:27 PM

Build is green

Patch application report for D8515 (id=30672)

Rebasing onto d445ce72ba...

Current branch diff-target is up to date.
Changes applied before test
commit 32d5564f9724c69e11cf9d7ffb53578cfaf55bae
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Wed Sep 21 11:36:00 2022 +0200

    Add filters for latest visit status
    
    Possibility to filter a latest vist status by status state and
    require snapshot.

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

This revision was automatically updated to reflect the committed changes.