Page MenuHomeSoftware Heritage
Differential D5788

common/origin_save: Update missing information when available
ClosedPublic
Actions

Authored by ardumont on May 27 2021, 11:35 AM.

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T3346: save code now: Some entries are missing information after update
Commits
rDWAPPS15168bc3cca7: common/origin_save: Update missing information when available
Summary

Due to some internal details [1], it happens that save code now requests are updated
with missing information. Broadening a bit the filtering on the save code now requests
to list for update, we can try and update those information more frequently (even when
the main information is already updated). This way, the save code now requests will
eventually be updated fully when the information lateness subsides.

[1] Replication lag

Related to T3346

Test Plan

tox

Diff Detail

Repository
rDWAPPS Web applications
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 21658
Build 33661: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 33660: arc lint + arc unit

Event Timeline

ardumont created this revision.May 27 2021, 11:35 AM
swh-public-ci added a comment.May 27 2021, 11:36 AM

Build has FAILED

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

Harbormaster returned this revision to the author for changes because remote builds failed.May 27 2021, 11:36 AM
Harbormaster failed remote builds in B21651: Diff 20689!
ardumont updated this revision to Diff 20690.May 27 2021, 11:42 AM

Might as well compare the date since we have it nearby

ardumont updated this revision to Diff 20692.May 27 2021, 11:54 AM

Update date manipulation

swh-public-ci added a comment.May 27 2021, 12:04 PM

Build has FAILED

Patch application report for D5788 (id=20690)

Rebasing onto 21616f5635...

Current branch diff-target is up to date.
Changes applied before test
commit c4318b034ddcc37daac58b2400189ac6e819d1be
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu May 27 11:25:06 2021 +0200

    common/origin_save: Update missing information when available
    
    Due to some internal details [1], it happens that save code now requests are updated
    with missing information. Broadening a bit the filtering on the save code now requests
    to list for update, we can try and update those information more frequently (even when
    the main information is already updated). This way, the save code now requests will
    eventually be updated fully when the information lateness subsides.
    
    [1] Replication lag
    
    Related to T3346

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

Harbormaster failed remote builds in B21652: Diff 20690!May 27 2021, 12:04 PM
swh-public-ci added a comment.May 27 2021, 12:16 PM

Build has FAILED

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

Harbormaster returned this revision to the author for changes because remote builds failed.May 27 2021, 12:16 PM
Harbormaster failed remote builds in B21654: Diff 20692!
ardumont updated this revision to Diff 20696.May 27 2021, 2:14 PM

Fix missing test update

swh-public-ci added a comment.May 27 2021, 2:16 PM

Build was aborted

Patch application report for D5788 (id=20692)

Rebasing onto 21616f5635...

Current branch diff-target is up to date.
Changes applied before test
commit ae4b69078661057149782965ff60a863b219d8f4
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu May 27 11:25:06 2021 +0200

    common/origin_save: Update missing information when available
    
    Due to some internal details [1], it happens that save code now requests are updated
    with missing information. Broadening a bit the filtering on the save code now requests
    to list for update, we can try and update those information more frequently (even when
    the main information is already updated). This way, the save code now requests will
    eventually be updated fully when the information lateness subsides.
    
    [1] Replication lag
    
    Related to T3346

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

swh-public-ci added a comment.May 27 2021, 2:38 PM

Build is green

Patch application report for D5788 (id=20696)

Rebasing onto 21616f5635...

Current branch diff-target is up to date.
Changes applied before test
commit 15168bc3cca7aaca24240917681cab937c25235d
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Thu May 27 11:25:06 2021 +0200

    common/origin_save: Update missing information when available
    
    Due to some internal details [1], it happens that save code now requests are updated
    with missing information. Broadening a bit the filtering on the save code now requests
    to list for update, we can try and update those information more frequently (even when
    the main information is already updated). This way, the save code now requests will
    eventually be updated fully when the information lateness subsides.
    
    [1] Replication lag
    
    Related to T3346

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

Harbormaster completed remote builds in B21658: Diff 20696.May 27 2021, 2:38 PM
ardumont requested review of this revision.May 27 2021, 2:38 PM
anlambert accepted this revision.May 27 2021, 2:45 PM

Looks good to me.

This revision is now accepted and ready to land.May 27 2021, 2:45 PM
Closed by commit rDWAPPS15168bc3cca7: common/origin_save: Update missing information when available (authored by ardumont). · Explain WhyMay 27 2021, 2:47 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDWAPPS15168bc3cca7: common/origin_save: Update missing information when available.
ardumont added inline comments.May 31 2021, 3:47 PM
swh/web/common/origin_save.py
615

Those 2 are wrongs, this should be an OR between those 2...

F(visit_date__isnull=True)|F(visit_status__isnull=True)
anlambert added inline comments.May 31 2021, 4:02 PM
swh/web/common/origin_save.py
615

Ah right, the visit date can be None while visit status is set according to that code.
Anyway this is an edge case as save code now reporting looks correct in production.
Django Q objects should be used to build the query then.

ardumont added a subscriber: vsellier.May 31 2021, 4:09 PM
ardumont added inline comments.
swh/web/common/origin_save.py
615

Thanks ;)

@vsellier noticed some misshaped save code now requests, that should be the reason ^ (heads up ;)

anlambert added inline comments.May 31 2021, 4:52 PM
swh/web/common/origin_save.py
615

I messed up the code link in my previous comment, here is the good one.

vsellier mentioned this in T3355: Running save code now request are never detected as completed by the webapp .Jun 2 2021, 11:57 AM
ardumont added inline comments.Jun 2 2021, 12:09 PM
swh/web/common/origin_save.py
615

@vsellier took care of it in D5810 \o/

douardda added a child revision: D7589: Add a --margin option to the `swh dataset graph export` command.Apr 15 2022, 12:29 PM
douardda removed a child revision: D7589: Add a --margin option to the `swh dataset graph export` command.Apr 15 2022, 12:55 PM

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
web/
common/
25 lines94%
Loading...
tests/
common/
67 lines100%
Loading...

Diff 20696

View Options

swh/web/common/origin_save.py

from typing import Dict, List, Optional, Union from typing import Dict, List, Optional, Union
import yaml
from swh.indexer.codemeta import CROSSWALK_TABLE, SCHEMA_URI from swh.indexer.codemeta import CROSSWALK_TABLE, SCHEMA_URI
from .base import DictMapping, SingleFileMapping from .base import YamlMapping
class SafeLoader(yaml.SafeLoader):
yaml_implicit_resolvers = {
k: [r for r in v if r[0] != "tag:yaml.org,2002:timestamp"]
for k, v in yaml.SafeLoader.yaml_implicit_resolvers.items()
}
class CffMapping(DictMapping, SingleFileMapping): class CffMapping(YamlMapping):
"""Dedicated class for Citation (CITATION.cff) mapping and translation""" """Dedicated class for Citation (CITATION.cff) mapping and translation"""
name = "cff" name = "cff"
filename = b"CITATION.cff" filename = b"CITATION.cff"
mapping = CROSSWALK_TABLE["Citation File Format Core (CFF-Core) 1.0.2"] mapping = CROSSWALK_TABLE["Citation File Format Core (CFF-Core) 1.0.2"]
string_fields = ["keywords", "license", "abstract", "version", "doi"] string_fields = ["keywords", "license", "abstract", "version", "doi"]
def translate(self, raw_content: bytes) -> Optional[Dict[str, str]]:
raw_content_string: str = raw_content.decode()
try:
content_dict = yaml.load(raw_content_string, Loader=SafeLoader)
except yaml.scanner.ScannerError:
return None
if isinstance(content_dict, dict):
return self._translate_dict(content_dict)
vlorentzUnsubmitted
Done
Inline Actions

please rebase before submitting diffs; this code was simplified; so you can simplify the new base YamlMapping similarly

vlorentz: please rebase before submitting diffs; this code was simplified; so you can simplify the new…
return None
def normalize_authors(self, d: List[dict]) -> Dict[str, list]: def normalize_authors(self, d: List[dict]) -> Dict[str, list]:
result = [] result = []
for author in d: for author in d:
author_data: Dict[str, Optional[Union[str, Dict]]] = { author_data: Dict[str, Optional[Union[str, Dict]]] = {
"@type": SCHEMA_URI + "Person" "@type": SCHEMA_URI + "Person"
} }
if "orcid" in author and isinstance(author["orcid"], str): if "orcid" in author and isinstance(author["orcid"], str):
author_data["@id"] = author["orcid"] author_data["@id"] = author["orcid"]
Show All 30 Lines
View Options

swh/web/tests/common/test_origin_save.py

from typing import Dict, List, Optional, Union from typing import Dict, List, Optional, Union
import yaml
from swh.indexer.codemeta import CROSSWALK_TABLE, SCHEMA_URI from swh.indexer.codemeta import CROSSWALK_TABLE, SCHEMA_URI
from .base import DictMapping, SingleFileMapping from .base import YamlMapping
class SafeLoader(yaml.SafeLoader):
yaml_implicit_resolvers = {
k: [r for r in v if r[0] != "tag:yaml.org,2002:timestamp"]
for k, v in yaml.SafeLoader.yaml_implicit_resolvers.items()
}
class CffMapping(DictMapping, SingleFileMapping): class CffMapping(YamlMapping):
"""Dedicated class for Citation (CITATION.cff) mapping and translation""" """Dedicated class for Citation (CITATION.cff) mapping and translation"""
name = "cff" name = "cff"
filename = b"CITATION.cff" filename = b"CITATION.cff"
mapping = CROSSWALK_TABLE["Citation File Format Core (CFF-Core) 1.0.2"] mapping = CROSSWALK_TABLE["Citation File Format Core (CFF-Core) 1.0.2"]
string_fields = ["keywords", "license", "abstract", "version", "doi"] string_fields = ["keywords", "license", "abstract", "version", "doi"]
def translate(self, raw_content: bytes) -> Optional[Dict[str, str]]:
raw_content_string: str = raw_content.decode()
try:
content_dict = yaml.load(raw_content_string, Loader=SafeLoader)
except yaml.scanner.ScannerError:
return None
if isinstance(content_dict, dict):
return self._translate_dict(content_dict)
vlorentzUnsubmitted
Done
Inline Actions

please rebase before submitting diffs; this code was simplified; so you can simplify the new base YamlMapping similarly

vlorentz: please rebase before submitting diffs; this code was simplified; so you can simplify the new…
return None
def normalize_authors(self, d: List[dict]) -> Dict[str, list]: def normalize_authors(self, d: List[dict]) -> Dict[str, list]:
result = [] result = []
for author in d: for author in d:
author_data: Dict[str, Optional[Union[str, Dict]]] = { author_data: Dict[str, Optional[Union[str, Dict]]] = {
"@type": SCHEMA_URI + "Person" "@type": SCHEMA_URI + "Person"
} }
if "orcid" in author and isinstance(author["orcid"], str): if "orcid" in author and isinstance(author["orcid"], str):
author_data["@id"] = author["orcid"] author_data["@id"] = author["orcid"]
Show All 30 Lines
About · Developer information · Legal