Page MenuHomeSoftware Heritage

Change the input type in contentByHashes entrypoint
ClosedPublic

Authored by jayeshv on Nov 29 2022, 12:16 PM.

Details

Summary

contentByHashes now accepts four different hashes instead of a list.
At least one hash must be provided for this entrypoint to work.

Returning a list of contents instead of an ambiguous single one is not
handled in this diff. That change requires some generic fixes and will be
addressed in another diff.

Diff Detail

Repository
rDGQL GraphQL API
Branch
use-content-hash-D8897
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 33228
Build 52098: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 52097: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D8897 (id=32065)

Rebasing onto c28b65feaf...

Current branch diff-target is up to date.
Changes applied before test
commit 3dd6f4ef206c34665bafb6bd86efdb9b58af1583
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Nov 29 12:06:59 2022 +0100

    Change the input type in contentByHashes entrypoint
    
    contentByHashes now accepts four different hashes instead of a list.
    At least one hash must be provided for this entrypoint to work.

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

lunar added inline comments.
swh/graphql/schema/schema.graphql
1083–1084

As this API might be surprising at first, I think it’s worth being explicit on the problem it is trying to solve.

swh/graphql/tests/functional/test_content.py
64–79

Was this one really meant to be part of this diff?

131–132

This is a bit confusing. I read this test as checking the behavior when only one hash over multiple match objects in the archive. I don’t believe it’s “wrong” as the request is just looking for an unknown object. Do you see what I mean? Maybe it should be the test should be labelled differently. Especially given passing the content of a blake2 hash as a sha_git feels indeed wrong. Maybe use an explicit aaaaaa… of the right length instead?

jayeshv added inline comments.
swh/graphql/schema/schema.graphql
1083–1084

Added a bit bigger description. This will change again if we decide to return a list of contents instead of a single one.

swh/graphql/tests/functional/test_content.py
64–79

This is not newly added or updated. Just grouped all the hash related tests, and this got placed in the top.

131–132

renamed the test and now using a different test hash

Build is green

Patch application report for D8897 (id=32067)

Rebasing onto c28b65feaf...

Current branch diff-target is up to date.
Changes applied before test
commit bacbdcd945fb804496a1dd996343e251ab71fd5b
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Nov 29 12:06:59 2022 +0100

    Change the input type in contentByHashes entrypoint
    
    contentByHashes now accepts four different hashes instead of a list.
    At least one hash must be provided for this entrypoint to work.

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

This revision is now accepted and ready to land.Dec 15 2022, 10:07 AM
jayeshv marked 3 inline comments as done.

rebase

Build is green

Patch application report for D8897 (id=32273)

Rebasing onto f1622d9c68...

Current branch diff-target is up to date.
Changes applied before test
commit c664ef42e7e6fff8b1a957502d9e0153869d72cf
Author: Jayesh Velayudhan <jayesh@softwareheritage.org>
Date:   Tue Nov 29 12:06:59 2022 +0100

    Change the input type in contentByHashes entrypoint
    
    contentByHashes now accepts four different hashes instead of a list.
    At least one hash must be provided for this entrypoint to work.

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