⚙ D5875 Add an endpoint to list and access raw extrinsic metadata.

vlorentz created this revision.Jun 15 2021, 4:50 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJun 15 2021, 4:50 PM

Build has FAILED

Patch application report for D5875 (id=21045)

Rebasing onto 01ffb31ff7...

First, rewinding head to replay your work on top of it...
Applying: Add an endpoint to list and access raw extrinsic metadata.

Changes applied before test

commit 4d286468c1296869e361a96c36e2313fb671e54d
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Jun 15 16:49:53 2021 +0200

    Add an endpoint to list and access raw extrinsic metadata.

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

Harbormaster returned this revision to the author for changes because remote builds failed.Jun 15 2021, 4:52 PM

Harbormaster failed remote builds in B22038: Diff 21045!

vlorentz requested review of this revision.Jun 15 2021, 5:00 PM

vlorentz edited the test plan for this revision. (Show Details)

looks rather good to me, couple of suggestions/remarks/questions inline.

swh/web/api/views/metadata.py
24	do you need to mention the swhid in the url? I don't really know if it's compatible with the other suggestion on the url below. That would match what we do with the content api (the raw endpoint).
60	maybe "trim" the authority_str because that could pass with malformed " authority-str" or some other form?
132	to match what we do with content...
swh/web/common/converters.py
299–307	there, compacted ;)

vlorentz added inline comments.Jun 17 2021, 10:07 AM

swh/web/api/views/metadata.py
24	I want to add origins next, so I wanted to differentiate the two URLs. That would match what we do with the content api (the raw endpoint). Which is exactly what I didn't want, actually. `/raw/` at the end of `"/metadata/extrinsic/(?P<target>{SWHID_RE})/raw/"` because it implies there is another endpoint `"/metadata/extrinsic/(?P<target>{SWHID_RE})/"`; but there isn't, and probably never will.

anlambert added a subscriber: anlambert.Jun 17 2021, 11:18 AM

anlambert added inline comments.

swh/web/api/views/metadata.py
1	Copyright (C) 2021
24	How about using `/metadata/extrinsic/(?P<target>{SWHID_RE})/` for that endpoint and `/metadata/extrinsic/(?P<target>{SWHID_RE})/raw` for the one getting the raw metadata bytes ? Raw implies a non JSON response format from my point of view.
42	You should add a description of the JSON response here.
70	why the `from None` here ?
144–147	return HttpResponse(...)

vlorentz added inline comments.Jun 17 2021, 12:09 PM

swh/web/api/views/metadata.py
24	but this endpoint is to get `raw_extrinsic_metadata` objects...
70	No reason in particular, I just don't see why we should attach the original traceback

anlambert added inline comments.Jun 17 2021, 1:54 PM

swh/web/api/views/metadata.py
27	Use the `swh.web.api.apidoc.apidoc` decorator here to register endpoint documentation. @api_doc("/metadata/extrinsic/raw/swhid/")
30	.. http:get:: /api/1/metadata/extrinsic/raw/swhid/(target)/
134	Use the `swh.web.api.apidoc.apidoc` decorator here to register endpoint documentation. @api_doc("/metadata/extrinsic/raw/get/")

vlorentz marked 7 inline comments as done.Jun 17 2021, 1:56 PM

vlorentz added inline comments.

swh/web/api/views/metadata.py
134	No, it's not documented on purpose because users shouldn't build this URL themselves

apply some comments
change URL scheme

rebase

Build has FAILED

Patch application report for D5875 (id=21082)

Rebasing onto 1bd55d031e...

First, rewinding head to replay your work on top of it...
Applying: Add an endpoint to list and access raw extrinsic metadata.

Changes applied before test

commit b34132f12f162d448cd7e1b2d6845c767d90c4c4
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Jun 15 16:49:53 2021 +0200

    Add an endpoint to list and access raw extrinsic metadata.

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

Harbormaster failed remote builds in B22070: Diff 21082!Jun 17 2021, 1:59 PM

Build has FAILED

Patch application report for D5875 (id=21083)

Rebasing onto 1bd55d031e...

Current branch diff-target is up to date.

Changes applied before test

commit 6cbe8079a4ed8a9f2742291b4c84f18e9f6e2eab
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Jun 15 16:49:53 2021 +0200

    Add an endpoint to list and access raw extrinsic metadata.

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

Harbormaster failed remote builds in B22071: Diff 21083!Jun 17 2021, 2:01 PM

add scroll test

Build has FAILED

Patch application report for D5875 (id=21117)

Rebasing onto 1bd55d031e...

Current branch diff-target is up to date.

Changes applied before test

commit 7c722d889d28d8c6fb2b4ed676fffa10452ad42c
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Jun 15 16:49:53 2021 +0200

    Add an endpoint to list and access raw extrinsic metadata.

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

Harbormaster failed remote builds in B22105: Diff 21117!Jun 18 2021, 11:23 AM

add tests for parameter checks

vlorentz edited the summary of this revision. (Show Details)Jun 18 2021, 11:38 AM

Build has FAILED

Patch application report for D5875 (id=21122)

Rebasing onto 1bd55d031e...

Current branch diff-target is up to date.

Changes applied before test

commit 4c22e06e79387cdc43a237c16a2cfd01777250ba
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Jun 15 16:49:53 2021 +0200

    Add an endpoint to list and access raw extrinsic metadata.

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

Harbormaster failed remote builds in B22110: Diff 21122!Jun 18 2021, 11:40 AM

anlambert added inline comments.Jun 18 2021, 11:46 AM

swh/web/api/views/metadata.py
74	You need to remove the leading slash here otherwise the following URL is generated ` /api/1//raw-extrinsic-metadata/swhid/swh:1:dir:a2faa28028657859c16ff506924212b33f0e1307/?authority=forge%20https://pypi.org/ ` which leads to a 404.

anlambert added inline comments.Jun 18 2021, 11:50 AM

swh/web/api/views/metadata.py

138

Pass the input request object as keyword parameter here, it will return a full URL.

Current JSON output is:

[
    {
        "authority": {
            "type": "forge",
            "url": "https://pypi.org/"
        },
        "discovery_date": "2021-05-07T04:31:14+00:00",
        "fetcher": {
            "name": "swh.loader.package.pypi.loader.PyPILoader",
            "version": "0.22.0"
        },
        "format": "pypi-project-json",
        "metadata_url": "/api/1/raw-extrinsic-metadata/get/107328dd41f23e8d4addf35a7091e435153a676e/",
        "origin": "https://pypi.org/project/swh.core/",
        "revision": "swh:1:rev:48371365859f4724c609895caaab362168623507",
        "target": "swh:1:dir:a2faa28028657859c16ff506924212b33f0e1307"
    }
]

anlambert added inline comments.Jun 18 2021, 11:56 AM

swh/web/api/views/metadata.py
174	How about setting a default filename for the blob to download as metadata id by adding that parameter ? headers={"Content-disposition": f"attachment; filename={id}"}

fix URL roots in documentation and returned objects, as requested

swh/web/api/views/metadata.py
174	I have a better idea; do you mind if I do it in a future diff?

Build has FAILED

Patch application report for D5875 (id=21256)

Rebasing onto 1bd55d031e...

Current branch diff-target is up to date.

Changes applied before test

commit 00f08cea038d6d5e1ed6998db4ba9c61b18ec399
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Jun 15 16:49:53 2021 +0200

    Add an endpoint to list and access raw extrinsic metadata.

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

Harbormaster failed remote builds in B22237: Diff 21256!Jun 25 2021, 11:07 AM

Looks good to me ! Time to tag swh-storage in order for the build to succeed.

This revision is now accepted and ready to land.Jun 25 2021, 11:11 AM

vlorentz added a child revision: D5921: Make api_raw_extrinsic_metadata_get set Content-disposition.Jun 25 2021, 11:16 AM

vlorentz added parent revisions: D5893: hypothesis_strategies: Add raw_extrinsic_metadata() strategy, D5865: Add endpoints to access REMD by id.Jun 25 2021, 11:23 AM

update deps

Build has FAILED

Patch application report for D5875 (id=21261)

Rebasing onto 1bd55d031e...

Current branch diff-target is up to date.

Changes applied before test

commit 785719a16a25bed9967c5086e781b77dc09e89f5
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Jun 15 16:49:53 2021 +0200

    Add an endpoint to list and access raw extrinsic metadata.

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

Harbormaster failed remote builds in B22242: Diff 21261!Jun 25 2021, 12:59 PM

bump deps again

vlorentz added a task: T2938: Create API endpoint to access raw_extrinsic_metadata.Jun 25 2021, 4:26 PM

Build is green

Patch application report for D5875 (id=21287)

Rebasing onto f9f1e969e9...

First, rewinding head to replay your work on top of it...
Applying: Add an endpoint to list and access raw extrinsic metadata.

Changes applied before test

commit 18632aae5f60e056a35d7932ec6ffb19f83f8252
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Jun 15 16:49:53 2021 +0200

    Add an endpoint to list and access raw extrinsic metadata.

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

Harbormaster completed remote builds in B22265: Diff 21287.Jun 25 2021, 4:35 PM

aea8ce1614421670f589dfe5a28b2750610a5a14

Add an endpoint to list and access raw extrinsic metadata.
ClosedPublic
Actions

Details

Diff Detail

Event Timeline

Patch application report for D5875 (id=21045)

Changes applied before test

Patch application report for D5875 (id=21082)

Changes applied before test

Patch application report for D5875 (id=21083)

Changes applied before test

Patch application report for D5875 (id=21117)

Changes applied before test

Patch application report for D5875 (id=21122)

Changes applied before test

Patch application report for D5875 (id=21256)

Changes applied before test

Patch application report for D5875 (id=21261)

Changes applied before test

Patch application report for D5875 (id=21287)

Changes applied before test

Revision Contents
Changeset List

Diff 21287

requirements-swh.txt

swh/web/api/urls.py

swh/web/api/views/metadata.py

swh/web/common/converters.py

swh/web/tests/api/views/test_metadata.py

swh/web/tests/api/views/test_origin.py

swh/web/tests/api/views/utils.py

swh/web/tests/common/test_converters.py

Add an endpoint to list and access raw extrinsic metadata.ClosedPublicActions

Details

Diff Detail

Event Timeline

Patch application report for D5875 (id=21045)

Changes applied before test

Patch application report for D5875 (id=21082)

Changes applied before test

Patch application report for D5875 (id=21083)

Changes applied before test

Patch application report for D5875 (id=21117)

Changes applied before test

Patch application report for D5875 (id=21122)

Changes applied before test

Patch application report for D5875 (id=21256)

Changes applied before test

Patch application report for D5875 (id=21261)

Changes applied before test

Patch application report for D5875 (id=21287)

Changes applied before test

Revision ContentsChangeset List

Diff 21287

requirements-swh.txt

swh/web/api/urls.py

swh/web/api/views/metadata.py

swh/web/common/converters.py

swh/web/tests/api/views/test_metadata.py

swh/web/tests/api/views/test_origin.py

swh/web/tests/api/views/utils.py

swh/web/tests/common/test_converters.py

Add an endpoint to list and access raw extrinsic metadata.
ClosedPublic
Actions

Revision Contents
Changeset List