Page MenuHomeSoftware Heritage
Differential D8878

vault/assets: Fix cooking task creation when there is a pending one
ClosedPublic
Actions

Authored by anlambert on Nov 23 2022, 4:50 PM.

Details

Reviewers
vlorentz
Group Reviewers
Reviewers
Maniphest Tasks
T4698: Error when requesting vault cooking for a directory in pending state
Commits
rDWAPPSc32edb667d1b: vault/assets: Fix cooking task creation when there is a pending one
Summary

When requesting a vault cooking creation but there is already a pending one
for the same SWHID, ensure task creation modal is displayed as previously an
error was erroneoulsy reported in the Web UI.




Under the hood, no new cooking task will be created but the pending one will

be displayed in the vault UI. Nevertheless the email possibly submitted by the

user in the modal will be added to the list of emails to notify by the vault

backend.






Related to T4698
Diff Detail
Repository 
rDWAPPS Web applications 
Branch 
vault-report-pending-request 
Lint 
No Linters Available
 
Unit 
No Unit Test Coverage
 
Build Status 
Buildable 32958
Build 51657: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 51656: arc lint + arc unit
 
Event Timeline
anlambert created this revision.Nov 23 2022, 4:50 PM
Herald added a reviewer: Reviewers.  ·  View Herald TranscriptNov 23 2022, 4:50 PM
swh-public-ci added a comment.Nov 23 2022, 5:06 PM
Build is green



Patch application report for D8878 (id=32008)



Could not rebase; Attempt merge onto b04d5848c6...



Updating b04d5848..a566268c
Fast-forward
 cypress/e2e/origin-save.cy.js              |  2 +-
 cypress/e2e/vault.cy.js                    | 19 +++++++++++++++++++
 swh/web/vault/assets/vault-create-tasks.js | 11 ++++++++++-
 3 files changed, 30 insertions(+), 2 deletions(-)





Changes applied before test



commit a566268c24f55d8334b49ea3801f4a281b9af3ca
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Report pending cooking task when submitting the same one
    
    Previously an error was erroneoulsy reported in the Web UI.
    
    Related to T4698

commit cd820470fa59192f318afc081d00e80c25fe2cc6
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 15:08:34 2022 +0100

    cypress/origin-save: Fix flaky test



See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/5/ for more details.
Harbormaster completed remote builds in B32941: Diff 32008.Nov 23 2022, 5:06 PM
anlambert requested review of this revision.Nov 23 2022, 5:06 PM
anlambert mentioned this in T4698: Error when requesting vault cooking for a directory in pending state.Nov 23 2022, 5:06 PM
vlorentz added a subscriber: vlorentz.Nov 24 2022, 9:59 AM
thanks!



Is it visible on the Downloads page even if someone else requested the cooking?
anlambert added a comment.Nov 24 2022, 10:39 AM


In D8878#230887, @vlorentz wrote:


thanks!



Is it visible on the Downloads page even if someone else requested the cooking?





Ah right, need to check.
anlambert added a comment.Nov 24 2022, 1:58 PM


In D8878#230891, @anlambert wrote:




In D8878#230887, @vlorentz wrote:


thanks!



Is it visible on the Downloads page even if someone else requested the cooking?





Ah right, need to check.





So the answer is no. It also makes me think that if a cooking task is pending, we still need to display the modal for creating a task as the user can ask for email notification.

I checked vault source code and when there is a pending cooking task, it will not be recreated but the provided email will be added to a list of emails to notify so fix is in fact

simpler than what I thought.
anlambert updated this revision to Diff 32020.Nov 24 2022, 2:29 PM
Rework fix to ensure cooking task will be displayed in vault UI.
anlambert retitled this revision from vault/assets: Report pending cooking task when submitting the same one to vault/assets: Fix cooking task creation when there is a pending one.Nov 24 2022, 2:30 PM
anlambert edited the summary of this revision. (Show Details)
vlorentz accepted this revision.Nov 24 2022, 2:32 PM
very nice!
This revision is now accepted and ready to land.Nov 24 2022, 2:32 PM
vlorentz requested changes to this revision.Nov 24 2022, 2:33 PM
vlorentz added inline comments.
swh/web/vault/assets/vault-create-tasks.js


52
wait, what happens if response.ok is true here? No feedback to the user?
This revision now requires changes to proceed.Nov 24 2022, 2:33 PM
swh-public-ci added a comment.Nov 24 2022, 2:43 PM
Build is green



Patch application report for D8878 (id=32020)



Rebasing onto cd820470fa...



Current branch diff-target is up to date.





Changes applied before test



commit f3b1786b639b1c9fa2b4b86eb18f87a3aa36b1be
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Fix cooking task creation when there is a pending one
    
    When requesting a vault cooking creation but there is already a pending one
    for the same SWHID, ensure task creation modal is displayed as previously an
    error was erroneoulsy reported in the Web UI.
    
    Under the hood, no new cooking task will be created but the pending one will
    be displayed in the vault UI. Nevertheless the email possibly submitted by the
    user in the modal will be added to the list of emails to notify by the vault
    backend.
    
    Related to T4698



See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/9/ for more details.
Harbormaster completed remote builds in B32955: Diff 32020.Nov 24 2022, 2:43 PM
anlambert updated this revision to Diff 32021.Nov 24 2022, 2:44 PM
Handle all possible cooking statuses.
anlambert added inline comments.Nov 24 2022, 2:48 PM
swh/web/vault/assets/vault-create-tasks.js


52
I updated the diff to handle all possible vault cooking statuses and the danger alert should only be displayed when HTTP response ends up with error so we should be good now.
swh-public-ci added a comment.Nov 24 2022, 2:53 PM
Build is green



Patch application report for D8878 (id=32021)



Rebasing onto cd820470fa...



Current branch diff-target is up to date.





Changes applied before test



commit c07f9ce462e8dba2df300ed66131bd32245436a3
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Fix cooking task creation when there is a pending one
    
    When requesting a vault cooking creation but there is already a pending one
    for the same SWHID, ensure task creation modal is displayed as previously an
    error was erroneoulsy reported in the Web UI.
    
    Under the hood, no new cooking task will be created but the pending one will
    be displayed in the vault UI. Nevertheless the email possibly submitted by the
    user in the modal will be added to the list of emails to notify by the vault
    backend.
    
    Related to T4698



See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/10/ for more details.
Harbormaster completed remote builds in B32956: Diff 32021.Nov 24 2022, 2:53 PM
vlorentz added inline comments.Nov 24 2022, 3:14 PM
swh/web/vault/assets/vault-create-tasks.js


52
Could you make it display "something unexpected happened" and send a Sentry report when none of the conditions is satisfied, then?



I don't like the idea of not giving any feedback when the backend sends an unexpected response
anlambert updated this revision to Diff 32022.Nov 24 2022, 3:30 PM
Update
anlambert added inline comments.Nov 24 2022, 3:33 PM
swh/web/vault/assets/vault-create-tasks.js


52
I removed the !response.ok condition. We do not have any caught exception here so I do not think it worth it to report to sentry as Web API errors are already reported.
swh-public-ci added a comment.Nov 24 2022, 3:34 PM
Build was aborted



Patch application report for D8878 (id=32022)



Rebasing onto cd820470fa...



Current branch diff-target is up to date.





Changes applied before test



commit e04412c408ece4e009f2360768ca04b31e05f537
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Fix cooking task creation when there is a pending one
    
    When requesting a vault cooking creation but there is already a pending one
    for the same SWHID, ensure task creation modal is displayed as previously an
    error was erroneoulsy reported in the Web UI.
    
    Under the hood, no new cooking task will be created but the pending one will
    be displayed in the vault UI. Nevertheless the email possibly submitted by the
    user in the modal will be added to the list of emails to notify by the vault
    backend.
    
    Related to T4698



Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/11/

See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/11/console
Harbormaster failed remote builds in B32957: Diff 32022!Nov 24 2022, 3:34 PM
anlambert updated this revision to Diff 32023.Nov 24 2022, 3:34 PM
Fix cypress test
swh-public-ci added a comment.Nov 24 2022, 3:50 PM
Build has FAILED



Patch application report for D8878 (id=32023)



Rebasing onto cd820470fa...



Current branch diff-target is up to date.





Changes applied before test



commit c32edb667d1b118ca973f5f3965a97b202ec80ea
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Fix cooking task creation when there is a pending one
    
    When requesting a vault cooking creation but there is already a pending one
    for the same SWHID, ensure task creation modal is displayed as previously an
    error was erroneoulsy reported in the Web UI.
    
    Under the hood, no new cooking task will be created but the pending one will
    be displayed in the vault UI. Nevertheless the email possibly submitted by the
    user in the modal will be added to the list of emails to notify by the vault
    backend.
    
    Related to T4698



Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/12/

See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/12/console
Harbormaster failed remote builds in B32958: Diff 32023!Nov 24 2022, 3:50 PM
vlorentz accepted this revision.Nov 24 2022, 3:57 PM
This revision is now accepted and ready to land.Nov 24 2022, 3:57 PM
swh-public-ci added a comment.Nov 24 2022, 4:29 PM
Build was aborted



Patch application report for D8878 (id=32023)



Rebasing onto cd820470fa...



Current branch diff-target is up to date.





Changes applied before test



commit c32edb667d1b118ca973f5f3965a97b202ec80ea
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Wed Nov 23 14:41:04 2022 +0100

    vault/assets: Fix cooking task creation when there is a pending one
    
    When requesting a vault cooking creation but there is already a pending one
    for the same SWHID, ensure task creation modal is displayed as previously an
    error was erroneoulsy reported in the Web UI.
    
    Under the hood, no new cooking task will be created but the pending one will
    be displayed in the vault UI. Nevertheless the email possibly submitted by the
    user in the modal will be added to the list of emails to notify by the vault
    backend.
    
    Related to T4698



Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/13/

See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/13/console
This revision was landed with ongoing or failed builds.Nov 24 2022, 4:48 PM
Closed by commit rDWAPPSc32edb667d1b: vault/assets: Fix cooking task creation when there is a pending one (authored by anlambert).  ·  Explain Why
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDWAPPSc32edb667d1b: vault/assets: Fix cooking task creation when there is a pending one.
swh-public-ci added a comment.Nov 24 2022, 5:06 PM
Build has FAILED



Patch application report for D8878 (id=32023)



Rebasing onto c32edb667d...



First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-14-D8878.





Changes applied before test






Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/14/

See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/14/console
swh-public-ci added a comment.Nov 24 2022, 5:14 PM
Build was aborted



Patch application report for D8878 (id=32023)



Rebasing onto c32edb667d...



First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-15-D8878.





Changes applied before test






Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/15/

See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/15/console
swh-public-ci added a comment.Nov 24 2022, 5:16 PM
Build has FAILED



Patch application report for D8878 (id=32023)



Rebasing onto c32edb667d...



First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-16-D8878.





Changes applied before test






Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/16/

See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/16/console
swh-public-ci added a comment.Nov 24 2022, 5:22 PM
Build was aborted



Patch application report for D8878 (id=32023)



Rebasing onto c32edb667d...



First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-17-D8878.





Changes applied before test






Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/17/

See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/17/console
swh-public-ci added a comment.Nov 24 2022, 5:29 PM
Build was aborted



Patch application report for D8878 (id=32023)



Rebasing onto c32edb667d...



First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-18-D8878.





Changes applied before test






Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/18/

See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/18/console
swh-public-ci added a comment.Nov 25 2022, 10:55 AM
Build was aborted



Patch application report for D8878 (id=32023)



Rebasing onto c32edb667d...



First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-19-D8878.





Changes applied before test






Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/19/

See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/19/console
swh-public-ci added a comment.Nov 25 2022, 11:29 AM
Build was aborted



Patch application report for D8878 (id=32023)



Rebasing onto c32edb667d...



First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-20-D8878.





Changes applied before test






Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/20/

See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/20/console
swh-public-ci added a comment.Nov 25 2022, 12:10 PM
Build was aborted



Patch application report for D8878 (id=32023)



Rebasing onto c32edb667d...



First, rewinding head to replay your work on top of it...
Fast-forwarded diff-target to base-revision-21-D8878.





Changes applied before test






Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/21/

See console output for more information: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/21/console
Revision Contents
Changeset List
PathSize
cypress/
e2e/
40 lines
swh/
web/
vault/
assets/
5 lines
Diff 32023
View Options
cypress/e2e/vault.cy.js
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/vault/assets/vault-create-tasks.js
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