Page MenuHomeSoftware Heritage
Differential D7174

Specify a new element to describe the provenance of deposit metadata
ClosedPublic
Actions

Authored by vlorentz on Feb 15 2022, 1:12 PM.

Details

Reviewers
moranegg
ardumont
Group Reviewers
Reviewers
Maniphest Tasks
T3677: Separate origin-source-code and provenance-metadata in the deposit
T3935: Change metadata-only deposits protocol to add a metadata-provenance url
Commits
rDDEPcc3705a3d33c: Specify a new element to describe the provenance of deposit metadata
Summary

This will be useful for metadata-only deposit, as there is not necessarily
and origin in the referenced SWHID; and even when there is, it is usually
not the actual source of the metadata.

Therefore, we need this new field to link back to the provenance of the metadata.

Related to T3677

Diff Detail

Repository
rDDEP Push deposit
Branch
metadata-provenance
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 27064
Build 42324: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 42323: arc lint + arc unit

Event Timeline

vlorentz created this revision.Feb 15 2022, 1:12 PM
swh-public-ci added a comment.Feb 15 2022, 1:15 PM

Build is green

Patch application report for D7174 (id=25999)

Rebasing onto c30dc3da46...

Current branch diff-target is up to date.
Changes applied before test
commit 4d123a5e0b305ceeafce1db7e51ebfbd6fa2db1e
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 15 13:12:08 2022 +0100

    Specify a new element to describe the provenance of deposit metadata
    
    This will be useful for metadata-only deposit, as there is not necessarily
    and origin in the referenced SWHID; and even when there is, it is usually
    not the actual source of the metadata.
    
    Therefore, we need this new field to link back to the provenance of the metadata.

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

Harbormaster completed remote builds in B26872: Diff 25999.Feb 15 2022, 1:15 PM
vlorentz requested review of this revision.Feb 15 2022, 1:15 PM
vlorentz updated this revision to Diff 26000.Feb 15 2022, 1:26 PM

fix namespace inconsistency wrt. the introduction

swh-public-ci added a comment.Feb 15 2022, 1:29 PM

Build is green

Patch application report for D7174 (id=26000)

Rebasing onto c30dc3da46...

Current branch diff-target is up to date.
Changes applied before test
commit 233363fddce5510f54731db4695fa098a3225a6f
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 15 13:12:08 2022 +0100

    Specify a new element to describe the provenance of deposit metadata
    
    This will be useful for metadata-only deposit, as there is not necessarily
    and origin in the referenced SWHID; and even when there is, it is usually
    not the actual source of the metadata.
    
    Therefore, we need this new field to link back to the provenance of the metadata.

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

Harbormaster completed remote builds in B26873: Diff 26000.Feb 15 2022, 1:29 PM
vlorentz updated this revision to Diff 26001.Feb 15 2022, 1:31 PM

missed one

swh-public-ci added a comment.Feb 15 2022, 1:34 PM

Build is green

Patch application report for D7174 (id=26001)

Rebasing onto c30dc3da46...

Current branch diff-target is up to date.
Changes applied before test
commit 9697301835a30302fe340c9b63c10a1bc6bd94d9
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 15 13:12:08 2022 +0100

    Specify a new element to describe the provenance of deposit metadata
    
    This will be useful for metadata-only deposit, as there is not necessarily
    and origin in the referenced SWHID; and even when there is, it is usually
    not the actual source of the metadata.
    
    Therefore, we need this new field to link back to the provenance of the metadata.

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

Harbormaster completed remote builds in B26874: Diff 26001.Feb 15 2022, 1:34 PM
vlorentz added a task: T3935: Change metadata-only deposits protocol to add a metadata-provenance url.Feb 15 2022, 2:55 PM
moranegg accepted this revision.Feb 15 2022, 3:35 PM

I tried checking the prov vocabulary is there is a metadata-provenance property and there is not.
This diff is very good and I like the explanation you have introduced, which is clear.
I have two questions in comments, which aren't blockers IMHO.

docs/specs/protocol-reference.rst
281

can we use <swh:metadata-provenance> and <swh:deposit> as introduced in the example below, instead of <swhdeposit:metadata-provenance> and <swhdeposit:deposit> ?

283

what is the schema.org namespace?

This revision is now accepted and ready to land.Feb 15 2022, 3:35 PM
vlorentz added inline comments.Feb 15 2022, 4:05 PM
docs/specs/protocol-reference.rst
281

This document already uses swhdeposit: in text and swh: in example to show the prefix does not matter; but I think I will change this in a future diff, because it is indeed confusing.

283

it's a synonym for "schema.org vocabulary", but I'm using the terminology used in XML.

moranegg added inline comments.Feb 15 2022, 4:37 PM
docs/specs/protocol-reference.rst
281

sure, in a future diff.
I know we have it elsewhere as well, but it might be confusing that it depends on the declaration of the namespace.

283

it's a synonym for "schema.org vocabulary", but I'm using the terminology used in XML.

yes right, I don't understand this specific context.
Is it just for the type url? do we use schema.org anywhere else?
BTW, I didn't find a provenance property in schema.org, maybe you had more luck.

vlorentz added inline comments.Feb 15 2022, 4:40 PM
docs/specs/protocol-reference.rst
283

yes, for now it's only for url, but I phrased it this way to be extensible.

provenance seems outside schema.org's scope. there's https://www.w3.org/ns/prov if you really want a dedicated vocabulary, but it's waaaay outside what we can use while remaining compatible with codemeta.

ardumont accepted this revision.Feb 15 2022, 4:45 PM

lgtm

one question regarding the xsd.

docs/specs/swh.xsd
39

Don't we want to be restrictive here first and then open more when we extend this (if we ever do it)?

vlorentz added inline comments.Feb 16 2022, 9:06 AM
docs/specs/swh.xsd
39

I don't think there is harm in allowing other metadata here. Plus, if there is any external reader of the metadata, they would want to future-proof it anyway.

ardumont added inline comments.Feb 18 2022, 2:00 PM
docs/specs/protocol-reference.rst
281–283

i think it's clearer. @vlorentz, @moranegg thoughts?

Also I'm more inclined towards imposing the namespace (last sentence). Because
otherwise, we could be annoyed by the various differences we'll be receiving and bug
will slip through.

If not ^, then the last sentence can be reworded as:

It's preferably defined using...
ardumont edited the summary of this revision. (Show Details)Feb 18 2022, 2:37 PM
ardumont added a task: T3677: Separate origin-source-code and provenance-metadata in the deposit.
vlorentz mentioned this in D7212: Fix URI of schema.org.Feb 21 2022, 5:07 PM
vlorentz updated this revision to Diff 26191.Feb 23 2022, 10:47 AM

rebase

This revision was landed with ongoing or failed builds.Feb 23 2022, 10:47 AM
Closed by commit rDDEPcc3705a3d33c: Specify a new element to describe the provenance of deposit metadata (authored by vlorentz). · Explain Why
This revision was automatically updated to reflect the committed changes.
vlorentz added a commit: rDDEPcc3705a3d33c: Specify a new element to describe the provenance of deposit metadata.
swh-public-ci added a comment.Feb 23 2022, 10:55 AM

Build is green

Patch application report for D7174 (id=26191)

Rebasing onto 55ae87b13c...

Current branch diff-target is up to date.
Changes applied before test
commit cc3705a3d33c07c7542024deb90d89fbc2c66853
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Feb 15 13:12:08 2022 +0100

    Specify a new element to describe the provenance of deposit metadata
    
    This will be useful for metadata-only deposit, as there is not necessarily
    and origin in the referenced SWHID; and even when there is, it is usually
    not the actual source of the metadata.
    
    Therefore, we need this new field to link back to the provenance of the metadata.

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

Harbormaster completed remote builds in B27064: Diff 26191.Feb 23 2022, 10:55 AM
ardumont mentioned this in T3935: Change metadata-only deposits protocol to add a metadata-provenance url.Feb 23 2022, 6:52 PM

Revision Contents
Changeset List

PathSize
docs/
specs/
73 lines
53 lines

Diff 26191

View Options

docs/specs/protocol-reference.rst

Show First 20 Lines • Show All 321 Lines • ▼ Show 20 Lines ) -> List[ListedOrigin]:
"""Get at most the `count` next origins that need to be visited with """Get at most the `count` next origins that need to be visited with
the `visit_type` loader according to the given scheduling `policy`. the `visit_type` loader according to the given scheduling `policy`.
This will mark the origins as "being visited" in the listed_origins This will mark the origins as "being visited" in the listed_origins
table, to avoid scheduling multiple visits to the same origin. table, to avoid scheduling multiple visits to the same origin.
""" """
origin_select_cols = ", ".join(ListedOrigin.select_columns()) origin_select_cols = ", ".join(ListedOrigin.select_columns())
# TODO: filter on last_scheduled "too recent" to avoid always
# re-scheduling the same tasks.
ardumontUnsubmitted
Not Done
Inline Actions

what does 'enabled' mean here?

I gather that ends up in the query like "WHERE enabled AND visit_type=%s"
but i don't know what that means exactly.

ardumont: what does 'enabled' mean here? I gather that ends up in the query like "WHERE enabled AND…
vlorentzAuthorUnsubmitted
Done
Inline Actions

Whether this origin has been seen during the last listing, and visits should be scheduled

vlorentz: Whether this origin has been seen during the last listing, and visits should be scheduled
olasdUnsubmitted
Not Done
Inline Actions

I don't think the enabled field is ever updated currently. But we will, eventually.

Obviously this would deserve a comment rather than being snuck in.

olasd: I don't think the `enabled` field is ever updated currently. But we will, eventually.
where_clauses = [
"enabled", # "NOT enabled" = the lister said the origin no longer exists
"visit_type = %s",
]
if policy == "oldest_scheduled_first": if policy == "oldest_scheduled_first":
query = f""" order_by = "origin_visit_stats.last_scheduled NULLS FIRST"
WITH selected_origins AS ( else:
raise UnknownPolicy(f"Unknown scheduling policy {policy}")
select_query = f"""
SELECT SELECT
{origin_select_cols} {origin_select_cols}
FROM FROM
listed_origins listed_origins
LEFT JOIN LEFT JOIN
origin_visit_stats USING (url, visit_type) origin_visit_stats USING (url, visit_type)
WHERE WHERE
visit_type = %s {" AND ".join(where_clauses)}
ORDER BY ORDER BY
origin_visit_stats.last_scheduled NULLS FIRST {order_by}
LIMIT %s LIMIT %s
"""
query = f"""
WITH selected_origins AS (
{select_query}
), ),
update_stats AS ( update_stats AS (
INSERT INTO INSERT INTO
origin_visit_stats ( origin_visit_stats (
url, visit_type, last_scheduled url, visit_type, last_scheduled
) )
SELECT SELECT
url, visit_type, now() url, visit_type, now()
FROM FROM
selected_origins selected_origins
ON CONFLICT (url, visit_type) DO UPDATE ON CONFLICT (url, visit_type) DO UPDATE
SET last_scheduled = GREATEST(origin_visit_stats.last_scheduled, EXCLUDED.last_scheduled) SET last_scheduled = GREATEST(
origin_visit_stats.last_scheduled,
EXCLUDED.last_scheduled
)
) )
SELECT SELECT
* *
FROM FROM
selected_origins selected_origins
""" # noqa """
cur.execute(query, (visit_type, count))
cur.execute(query, (visit_type, count))
return [ListedOrigin(**d) for d in cur] return [ListedOrigin(**d) for d in cur]
else:
raise UnknownPolicy(f"Unknown scheduling policy {policy}")
task_create_keys = [ task_create_keys = [
"type", "type",
"arguments", "arguments",
"next_run", "next_run",
"policy", "policy",
"status", "status",
"retries_left", "retries_left",
▲ Show 20 Lines • Show All 567 Lines • Show Last 20 Lines
View Options

docs/specs/swh.xsd

Show First 20 Lines • Show All 321 Lines • ▼ Show 20 Lines ) -> List[ListedOrigin]:
"""Get at most the `count` next origins that need to be visited with """Get at most the `count` next origins that need to be visited with
the `visit_type` loader according to the given scheduling `policy`. the `visit_type` loader according to the given scheduling `policy`.
This will mark the origins as "being visited" in the listed_origins This will mark the origins as "being visited" in the listed_origins
table, to avoid scheduling multiple visits to the same origin. table, to avoid scheduling multiple visits to the same origin.
""" """
origin_select_cols = ", ".join(ListedOrigin.select_columns()) origin_select_cols = ", ".join(ListedOrigin.select_columns())
# TODO: filter on last_scheduled "too recent" to avoid always
# re-scheduling the same tasks.
ardumontUnsubmitted
Not Done
Inline Actions

what does 'enabled' mean here?

I gather that ends up in the query like "WHERE enabled AND visit_type=%s"
but i don't know what that means exactly.

ardumont: what does 'enabled' mean here? I gather that ends up in the query like "WHERE enabled AND…
vlorentzAuthorUnsubmitted
Done
Inline Actions

Whether this origin has been seen during the last listing, and visits should be scheduled

vlorentz: Whether this origin has been seen during the last listing, and visits should be scheduled
olasdUnsubmitted
Not Done
Inline Actions

I don't think the enabled field is ever updated currently. But we will, eventually.

Obviously this would deserve a comment rather than being snuck in.

olasd: I don't think the `enabled` field is ever updated currently. But we will, eventually.
where_clauses = [
"enabled", # "NOT enabled" = the lister said the origin no longer exists
"visit_type = %s",
]
if policy == "oldest_scheduled_first": if policy == "oldest_scheduled_first":
query = f""" order_by = "origin_visit_stats.last_scheduled NULLS FIRST"
WITH selected_origins AS ( else:
raise UnknownPolicy(f"Unknown scheduling policy {policy}")
select_query = f"""
SELECT SELECT
{origin_select_cols} {origin_select_cols}
FROM FROM
listed_origins listed_origins
LEFT JOIN LEFT JOIN
origin_visit_stats USING (url, visit_type) origin_visit_stats USING (url, visit_type)
WHERE WHERE
visit_type = %s {" AND ".join(where_clauses)}
ORDER BY ORDER BY
origin_visit_stats.last_scheduled NULLS FIRST {order_by}
LIMIT %s LIMIT %s
"""
query = f"""
WITH selected_origins AS (
{select_query}
), ),
update_stats AS ( update_stats AS (
INSERT INTO INSERT INTO
origin_visit_stats ( origin_visit_stats (
url, visit_type, last_scheduled url, visit_type, last_scheduled
) )
SELECT SELECT
url, visit_type, now() url, visit_type, now()
FROM FROM
selected_origins selected_origins
ON CONFLICT (url, visit_type) DO UPDATE ON CONFLICT (url, visit_type) DO UPDATE
SET last_scheduled = GREATEST(origin_visit_stats.last_scheduled, EXCLUDED.last_scheduled) SET last_scheduled = GREATEST(
origin_visit_stats.last_scheduled,
EXCLUDED.last_scheduled
)
) )
SELECT SELECT
* *
FROM FROM
selected_origins selected_origins
""" # noqa """
cur.execute(query, (visit_type, count))
cur.execute(query, (visit_type, count))
return [ListedOrigin(**d) for d in cur] return [ListedOrigin(**d) for d in cur]
else:
raise UnknownPolicy(f"Unknown scheduling policy {policy}")
task_create_keys = [ task_create_keys = [
"type", "type",
"arguments", "arguments",
"next_run", "next_run",
"policy", "policy",
"status", "status",
"retries_left", "retries_left",
▲ Show 20 Lines • Show All 567 Lines • Show Last 20 Lines
About · Developer information · Legal