Page MenuHomeSoftware Heritage

docs/grpc-api.rst: Add Python examples
ClosedPublic

Authored by vlorentz on Oct 17 2022, 5:54 PM.

Event Timeline

Harbormaster returned this revision to the author for changes because remote builds failed.Oct 17 2022, 5:55 PM
Harbormaster failed remote builds in B32334: Diff 31387!

Build is green

Patch application report for D8691 (id=31387)

Could not rebase; Attempt merge onto b5ea368cd4...

Updating b5ea368..6ce981a
Fast-forward
 docs/grpc-api.rst | 286 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 280 insertions(+), 6 deletions(-)
Changes applied before test
commit 6ce981a670e2fc805e99bddfc7860483f4f5e70a
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 17 17:53:24 2022 +0200

    docs/grpc-api.rst: Add Python examples

commit ae67cd1cd01e2569658c3a36257250f3dee93f7c
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 17 17:51:21 2022 +0200

    docs/grpc-api.rst: Update to match to current code

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

anlambert added a subscriber: anlambert.

Looks good to me.

docs/grpc-api.rst
102

add spaces around * operator

This revision is now accepted and ready to land.Oct 18 2022, 4:08 PM
ardumont added a subscriber: ardumont.

lgtm

couple of suggestions inline.

docs/grpc-api.rst
169

"maybe" we could suggest a breakpoint (with pdbpp) or ipython in the code (as a note below) so people could investigate responseby themselves too.

response.<tab><tab>...

351

You may want to mention that the "paths" values (in the request) are actually what's going to be mapped as a response (in the generic context you mentioned early). That seems to be a pattern from afar.

docs/grpc-api.rst
169

meh, this is going to bloat the examples

351

Isn't this already documented above?

A FieldMask is represented as a set of "field paths" in dotted notation. For
instance, ``paths: ["swhid", "rev.message"]`` will only request the swhid and
the message of a given node. An empty mask will return an empty object.
docs/grpc-api.rst
169

I meant as a one-off note in the main part, with the context at the beginning of the sampled python (not for all python examples...).
But fine.

351

No, i don't think that it is that explicit.
But i'm fine either way. Like i said (non-blocking) suggestions.

olasd added inline comments.
docs/grpc-api.rst
304–308

This makes me wonder whether the masks should be made mandatory (or default to an empty value, instead of the current "maximalist" default).

repeat semantics of FieldMasks in Python examples

vlorentz marked 3 inline comments as done.

Blacken example

docs/grpc-api.rst
304–308

Omitting masks is very convenient while debugging (eg. on a CLI) so we shouldn't do it on the server side IMO. And to do it on the client side, we would need a wrapper class in each language, which is meh.

Build is green

Patch application report for D8691 (id=31557)

Rebasing onto 647e82bed7...

First, rewinding head to replay your work on top of it...
Applying: docs/grpc-api.rst: Update to match to current code
Applying: docs/grpc-api.rst: Add Python examples
Changes applied before test
commit 08f38dd5e5f5c9f8e1910dc26808aa325de7858b
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 17 17:53:24 2022 +0200

    docs/grpc-api.rst: Add Python examples

commit 6034addd18bd89b66fccb33e3ba9cb44dc1198de
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 17 17:51:21 2022 +0200

    docs/grpc-api.rst: Update to match to current code

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

Build is green

Patch application report for D8691 (id=31558)

Rebasing onto 647e82bed7...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-273-D8691.
Changes applied before test

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

Build is green

Patch application report for D8691 (id=31559)

Could not rebase; Attempt merge onto 647e82bed7...

Merge made by the 'recursive' strategy.
 docs/grpc-api.rst | 288 ++++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 282 insertions(+), 6 deletions(-)
Changes applied before test
commit b6bb349d8ffb54b56eda2b1773774698ae6d4465
Merge: 647e82b 984b1b5
Author: Jenkins user <jenkins@localhost>
Date:   Mon Oct 24 08:00:19 2022 +0000

    Merge branch 'diff-target' into HEAD

commit 984b1b545ec72f2895be802a403e88018dc20b32
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 17 17:53:24 2022 +0200

    docs/grpc-api.rst: Add Python examples

commit ae67cd1cd01e2569658c3a36257250f3dee93f7c
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Mon Oct 17 17:51:21 2022 +0200

    docs/grpc-api.rst: Update to match to current code

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

This revision was landed with ongoing or failed builds.Oct 24 2022, 10:33 AM
This revision was automatically updated to reflect the committed changes.

Build has FAILED

Patch application report for D8691 (id=31561)

Rebasing onto edcca81a60...

First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-276-D8691.
Changes applied before test

Link to build: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/276/
See console output for more information: https://jenkins.softwareheritage.org/job/DGRPH/job/tests-on-diff/276/console