Paths

Table of Contentst

Differential D8897

Change the input type in contentByHashes entrypoint
ClosedPublic
Actions

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

Details

Reviewers

vlorentz

Group Reviewers

Reviewers

Commits

rDGQLc664ef42e7e6: Change the input type in contentByHashes entrypoint

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

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jayeshv created this revision.Nov 29 2022, 12:16 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptNov 29 2022, 12:16 PM

jayeshv edited the summary of this revision. (Show Details)Nov 29 2022, 12:18 PM

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.

Harbormaster completed remote builds in B33011: Diff 32065.Nov 29 2022, 12:18 PM

jayeshv requested review of this revision.Nov 29 2022, 12:18 PM

lunar added a subscriber: lunar.Nov 29 2022, 12:31 PM

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?

address review comments

jayeshv marked 3 inline comments as done.Nov 29 2022, 1:34 PM

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.

Harbormaster completed remote builds in B33013: Diff 32067.Nov 29 2022, 1:34 PM

vlorentz accepted this revision.Dec 15 2022, 10:07 AM

This revision is now accepted and ready to land.Dec 15 2022, 10:07 AM

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.

Harbormaster completed remote builds in B33228: Diff 32273.Dec 15 2022, 10:20 AM

Closed by commit rDGQLc664ef42e7e6: Change the input type in contentByHashes entrypoint (authored by jayeshv). · Explain WhyDec 15 2022, 10:21 AM

This revision was automatically updated to reflect the committed changes.

jayeshv added a commit: rDGQLc664ef42e7e6: Change the input type in contentByHashes entrypoint.