Differential D7972

Support empty response from swh-graph
ClosedPublic
Actions

Authored by vsellier on Jun 8 2022, 1:59 PM.

Details

Reviewers

ardumont

Group Reviewers

Reviewers

Maniphest Tasks

T4314: [provenance] HistoryGraph building is failing when reaching the first revision

Commits

rDPROVd6e88dabb3bc: Support empty response from swh-graph

Summary

Workaround to support the empty response of swh-graph when a
revision has no parent

In this case, swh-graph should reply a 201 no-content
and this response should be correctly interpreted by the swh-graph
lib, but it the meantime, it'll allow provenance to work with
swh-graph

Related to T4314

Diff Detail

Repository

rDPROV Provenance database

Lint

Automatic diff as part of commit; lint not applicable.

Unit

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

Event Timeline

vsellier created this revision.Jun 8 2022, 1:59 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJun 8 2022, 1:59 PM

Build is green

Patch application report for D7972 (id=28720)

Rebasing onto 5cbbb9a333...

Current branch diff-target is up to date.

Changes applied before test

commit d6e88dabb3bcbb69b6701f5dc1115a998c5c2bff
Author: Vincent SELLIER <vincent.sellier@softwareheritage.org>
Date:   Wed Jun 8 13:56:09 2022 +0200

    Support empty response from swh-graph
    
    Workaround to support the empty response of swh-graph when a
    revision has no parent
    
    In this case, swh-graph should reply a 201 no-content
    and this response should be correctly interpreted by the swh-graph
    lib, but it the meantime, it'll allow provenance to work with
    swh-graph
    
    Related to T4314

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

Harbormaster completed remote builds in B29801: Diff 28720.Jun 8 2022, 2:07 PM

vsellier requested review of this revision.Jun 8 2022, 2:07 PM

this looks like a bug in swh-graph; it shouldn't return empty lines

sorry, I mean: why not fix it directly in swh-graph, instead of adding a workaround here?

Because it looks simpler and faster to have this workaround here.

Don't hesitate to open an issue in the graph if you think it's worth it, this workaround doesn't avoid to fix it also in the graph later.

For the record, we need need to speed up ASAP the origin processing of provenance so it looked as a good compromise (cc @bchauvet).

this looks like a bug in swh-graph; it shouldn't return empty lines

(forgot to hit enter)

It is but that won't be fixed immediately [1]

[1] (edited)

14:30 <+vsellier> that plus the fact the swh-graph python api is not correctly dealing with the result of the api when a revision is not existing
14:31 <+vsellier> returns a 400 Bad request
14:32 <+seirl> yeah, that'll be fixed with the grpc switch
...
14:42 <+ardumont> seirl: when does the grpc switch is supposed to happen though?
14:42 <+ardumont> context is discussion on D7972
14:42 -- Notice(swhbot): D7972 (author: vsellier, Needs Review) on swh-provenance: Support empty response from swh-graph <https://forge.softwareheritage.org/D7972>
14:42 <+ardumont> val ^
14:43 <+seirl> ardumont: next week i'd say? i have all the code and all the tests, i just need to make the CI pass
14:43 <+seirl> and to add some doc
14:43 <+seirl> the draft diff is already available

lg "enough a workaround" tm for now.

This revision is now accepted and ready to land.Jun 8 2022, 3:07 PM

Closed by commit rDPROVd6e88dabb3bc: Support empty response from swh-graph (authored by vsellier). · Explain WhyJun 8 2022, 11:50 PM

This revision was automatically updated to reflect the committed changes.

vsellier added a commit: rDPROVd6e88dabb3bc: Support empty response from swh-graph.

vsellier mentioned this in T4314: [provenance] HistoryGraph building is failing when reaching the first revision.Jun 8 2022, 11:51 PM