Names got changed and, unfortunately, it was not caught neither by vault tests nor swh-web's ¯\_(ツ)_/¯.
wip and incomplete (needs to fix tests now)
In docker, i no longer reproduce with this.
Differential D4521
vault: Fix vault response schema ardumont on Nov 19 2020, 11:42 AM. Authored by
Details
Names got changed and, unfortunately, it was not caught neither by vault tests nor swh-web's ¯\_(ツ)_/¯. wip and incomplete (needs to fix tests now) In docker, i no longer reproduce with this. tox docker $ cd swh-web && yarn build-dev $ emacs docker-compose.override.yml # and add the swh-web override mount points with the local swh-web repository $ docker-compose up $ # load some repository $ # request some cooking $ # check the listing now works
Diff Detail
Event TimelineComment Actions Build has FAILED Patch application report for D4521 (id=16022)Rebasing onto 660ef62fd6... First, rewinding head to replay your work on top of it... Applying: vault-ui: Fix vault listing Changes applied before testcommit a967e789bd61a4e5433bfde392a5e0d61e1c628a Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Nov 19 11:12:48 2020 +0100 vault-ui: Fix vault listing Names got changed Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/476/ Comment Actions Technically viable! with docker [1], it works! [1] $ cd swh-web && yarn build-dev $ emacs docker-compose.override.yml # and add the swh-web with the repository $ docker-compose up do some cooking and check the listing Comment Actions Either that or modifying the vault so it keeps the same output as before or modifying the api in the webapp which makes a transformation of the data coming from the vault. Comment Actions Thanks for fixing this. I also think we could add a commit to print possible future errors by turning the catch handler to something like .catch(error => { console.log('Error when fetching vault cooking tasks:', error); }); We could also add an error message to display in place of the empty cooking list. Comment Actions yep, good idea, it'd be nice indeed to have some feedback there. It hit me at some point but it was more like shot in the dark debugging ;) Comment Actions
Comment Actions Build has FAILED Patch application report for D4521 (id=16033)Rebasing onto 660ef62fd6... Current branch diff-target is up to date. Changes applied before testcommit 014e729daee4aa6d84bea1d8caf760b3abcf4e9f Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Nov 19 13:56:37 2020 +0100 vault-ui: Log caught error when listing commit 9ae89e87cd181c73fa94a9f3e9a5131031198ab6 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Nov 19 11:12:48 2020 +0100 vault: Fix vault listing ui Names got changed in the api backend. Unfortunately, this passed undetected both by the vault and the webapp tests. As the try catch within the js is silent, this fails silently. This commit fixes to use the right record names. Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/477/ Comment Actions as discussed with @anlambert, it'd be best to have the change python side to avoid client side behavior change. Comment Actions Yes, after thinking back on the issue, we cannot introduce breaking changes in the public Web API responses so the changes of Comment Actions Fix issue python side Comment Actions Build has FAILED Patch application report for D4521 (id=16045)Rebasing onto 660ef62fd6... Current branch diff-target is up to date. Changes applied before testcommit 714d628f683f393288e7fdc672750463aee6123e Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Nov 19 14:23:06 2020 +0100 vault: Fix vault listing ui Names got changed in the api backend. Unfortunately, this passed undetected both by the vault and the webapp tests. As the try catch within the js is silent, this fails silently. This commit fixes to create a mapping from the new names to the old names to keep the api consistent with existing behavior. commit ef24a558eb63f1e88a7d223d7b7cd7a77e54d5a7 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Nov 19 13:56:37 2020 +0100 vault-ui: Log caught error when listing Link to build: https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/478/
Comment Actions Build is green Patch application report for D4521 (id=16056)Rebasing onto 660ef62fd6... Current branch diff-target is up to date. Changes applied before testcommit 0c12b5ca2864007b5b33eaff8370e0a4db13d211 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Nov 19 14:23:06 2020 +0100 vault: Fix vault listing ui Names got changed in the api backend. Unfortunately, this passed undetected both by the vault and the webapp tests. As the try catch within the js is silent, this fails silently. This commit fixes to create a mapping from the new names to the old names to keep the api consistent with existing behavior. commit ef24a558eb63f1e88a7d223d7b7cd7a77e54d5a7 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Nov 19 13:56:37 2020 +0100 vault-ui: Log caught error when listing See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/479/ for more details. Comment Actions Build is green Patch application report for D4521 (id=16059)Rebasing onto 660ef62fd6... Current branch diff-target is up to date. Changes applied before testcommit cd5a17950ad93ca8b7de6564c824d1768fb26a15 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Nov 19 14:23:06 2020 +0100 vault: Fix vault listing ui Names got changed in the api backend. Unfortunately, this passed undetected both by the vault and the webapp tests. As the try catch within the js is silent, this fails silently. This commit fixes to create a mapping from the new names to the old names to keep the api consistent with existing behavior. commit ef24a558eb63f1e88a7d223d7b7cd7a77e54d5a7 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Nov 19 13:56:37 2020 +0100 vault-ui: Log caught error when listing See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/480/ for more details. Comment Actions Just some improvements to bring to the fetch_url value and we should be good to land this.
Comment Actions Build is green Patch application report for D4521 (id=16060)Rebasing onto 660ef62fd6... Current branch diff-target is up to date. Changes applied before testcommit 6eb2ceb4689410d0ea274e0bdb62154bf8622b33 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Nov 19 14:23:06 2020 +0100 vault: Fix vault listing ui Names got changed in the api backend. Unfortunately, this passed undetected both by the vault and the webapp tests. As the try catch within the js is silent, this fails silently. This commit fixes to create a mapping from the new names to the old names to keep the api consistent with existing behavior. See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/481/ for more details. Comment Actions Build is green Patch application report for D4521 (id=16062)Rebasing onto 660ef62fd6... Current branch diff-target is up to date. Changes applied before testcommit 4d35a4435a2c0dd6fac69562985f8e3082ea1821 Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org> Date: Thu Nov 19 14:23:06 2020 +0100 vault: Fix vault response schema Names got changed in the vault api backend. Unfortunately, this passed undetected both by the vault and the webapp tests. Furthermore, as the try catch within the js layer is silent, this fails silently. This commit fixes the situation by mapping from the new names to the old names. This keeps the the api behavior consistent. See https://jenkins.softwareheritage.org/job/DWAPPS/job/tests-on-diff/483/ for more details. |