Page MenuHomeSoftware Heritage

vault API: Rename bundle types and use SWHIDs to identify objects
ClosedPublic

Authored by vlorentz on Aug 18 2021, 7:03 PM.

Details

Summary

This is to match changes in the vault interface, see D6112 for the rationale.

All existing queries still work, through "fallback" views. There are
some minor changes in returned fields (removed obj_type, and replace
obj_id with swhid), but I don't think anyone used them (since that info
is already available to them before they send the request).

Test Plan

Tests will fail until D6112 is released.

Diff Detail

Repository
rDWAPPS Web applications
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Build is green

Patch application report for D6113 (id=22118)

Rebasing onto 885be5dde5...

Current branch diff-target is up to date.
Changes applied before test
commit 483ea90504e2dacecb9dc955e3fc9c81951f014a
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 18 19:02:10 2021 +0200

    vault API: Rename bundle types and use SWHIDs to identify objects
    
    This is to match changes in the vault interface, see D6112 for the rationale.
    
    All existing queries still work, through "fallback" views.  There are
    some minor changes in returned fields (removed obj_type, and replace
    obj_id with swhid), but I don't think anyone used them (since that info
    is already available to them before they send the request).

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

Build was aborted

Patch application report for D6113 (id=22125)

Rebasing onto 885be5dde5...

Current branch diff-target is up to date.
Changes applied before test
commit ee80e42d86d3bac2635ca7de16018437afd7b271
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 18 19:02:10 2021 +0200

    vault API: Rename bundle types and use SWHIDs to identify objects
    
    This is to match changes in the vault interface, see D6112 for the rationale.
    
    All existing queries still work, through "fallback" views.  There are
    some minor changes in returned fields (removed obj_type, and replace
    obj_id with swhid), but I don't think anyone used them (since that info
    is already available to them before they send the request).

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

anlambert added inline comments.
assets/src/bundles/vault/vault-create-tasks.js
59–72

As the old JSON schema for cooking tasks is still used in production and stored in local browser storage, you should add backward compatibility for it otherwise errors will appear for users that already used the vault UI.

fix bugs, add test, add migration for client-side data

Build is green

Patch application report for D6113 (id=22160)

Could not rebase; Attempt merge onto 8acd726369...

Merge made by the 'recursive' strategy.
 assets/src/bundles/vault/vault-create-tasks.js     |  50 ++--
 assets/src/bundles/vault/vault-table-row.ejs       |  18 +-
 assets/src/bundles/vault/vault-ui.js               |  64 +++--
 ...d19126d815470b28919d64b2a8e6a3e37f900dd.tar.gz} | Bin
 ...a4573d2a003fc2630c21c2b25829de49972.gitfast.gz} | Bin
 cypress/integration/vault.spec.js                  | 145 ++++++++----
 swh/web/api/views/vault.py                         | 257 ++++++++++++++-------
 swh/web/browse/snapshot_context.py                 |   4 +-
 swh/web/browse/urls.py                             |   6 +-
 swh/web/browse/views/directory.py                  |   4 +-
 swh/web/browse/views/release.py                    |   8 +-
 swh/web/browse/views/revision.py                   |   6 +-
 swh/web/common/archive.py                          |  25 +-
 swh/web/templates/includes/vault-create-tasks.html |  22 +-
 swh/web/tests/api/views/test_vault.py              | 230 +++++++++++++++---
 15 files changed, 587 insertions(+), 252 deletions(-)
 rename cypress/fixtures/{cd19126d815470b28919d64b2a8e6a3e37f900dd.tar.gz => swh:1:dir:cd19126d815470b28919d64b2a8e6a3e37f900dd.tar.gz} (100%)
 rename cypress/fixtures/{1c480a4573d2a003fc2630c21c2b25829de49972.gitfast.gz => swh:1:rev:1c480a4573d2a003fc2630c21c2b25829de49972.gitfast.gz} (100%)
Changes applied before test
commit f51e17f0fb6cece0bae60272260e087132ed5d19
Merge: 8acd7263 b217cb75
Author: Jenkins user <jenkins@localhost>
Date:   Tue Aug 24 11:57:11 2021 +0000

    Merge branch 'diff-target' into HEAD

commit b217cb75e4466f64c6f9809d9c4eaabb858fd627
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 18 19:02:10 2021 +0200

    vault API: Rename bundle types and use SWHIDs to identify objects
    
    This is to match changes in the vault interface, see D6112 for the rationale.
    
    All existing queries still work, through "fallback" views.  There are
    some minor changes in returned fields (removed obj_type, and replace
    obj_id with swhid), but I don't think anyone used them (since that info
    is already available to them before they send the request).

commit e6463f5241a3da404ea5f60dd52807054f060449
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Aug 24 11:23:53 2021 +0200

    vault.spec.js: Add test checking the LocalStorage is used to show the task list

commit c8a1808415d796500ea16774db024a92dc050593
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Aug 24 10:19:22 2021 +0200

    vault.spec.js: Remove vaultItems from global variables
    
    So tests don't have side-effects

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

Looks good to me.

As we have existing Web API users using the vault endpoints in their code (Guix for instance),
I think we should send a message to swh-devel indicating that previous endpoints are deprecated
and encourage the use of new ones. Apidoc for deprecated endpoints should also remain visible for
some time I think.

assets/src/bundles/vault/vault-ui.js
142

remaining debug log here

cypress/integration/vault.spec.js
211–220

From my point of view, those intercepts can be removed as no requests will be sent as the cooking task in local storage has the done status plus the call to click below is not really needed.

swh/web/api/views/vault.py
140

Should we really hide that apidoc entry right now ? We have users that can still query that deprecated endpoint in their code.
Indicating the endpoint deprecation in the doc could be useful to them.

This revision is now accepted and ready to land.Aug 24 2021, 6:25 PM
vlorentz marked 2 inline comments as done.

remove debug log + remove useless code from tests + fix deprecation notices

swh/web/api/views/vault.py
140

tags=["hidden"] only hides it from the endpoint index; but the documentation (and deprecation notice) can still be accessed directly.

Build is green

Patch application report for D6113 (id=22179)

Could not rebase; Attempt merge onto 8acd726369...

Updating 8acd7263..c402ae6a
Fast-forward
 assets/src/bundles/vault/vault-create-tasks.js     |  50 ++--
 assets/src/bundles/vault/vault-table-row.ejs       |  18 +-
 assets/src/bundles/vault/vault-ui.js               |  63 +++--
 ...d19126d815470b28919d64b2a8e6a3e37f900dd.tar.gz} | Bin
 ...a4573d2a003fc2630c21c2b25829de49972.gitfast.gz} | Bin
 cypress/integration/vault.spec.js                  | 123 +++++++---
 swh/web/api/views/vault.py                         | 267 ++++++++++++++-------
 swh/web/browse/snapshot_context.py                 |   4 +-
 swh/web/browse/urls.py                             |   6 +-
 swh/web/browse/views/directory.py                  |   4 +-
 swh/web/browse/views/release.py                    |   8 +-
 swh/web/browse/views/revision.py                   |   6 +-
 swh/web/common/archive.py                          |  25 +-
 swh/web/templates/includes/vault-create-tasks.html |  22 +-
 swh/web/tests/api/views/test_vault.py              | 230 +++++++++++++++---
 15 files changed, 573 insertions(+), 253 deletions(-)
 rename cypress/fixtures/{cd19126d815470b28919d64b2a8e6a3e37f900dd.tar.gz => swh:1:dir:cd19126d815470b28919d64b2a8e6a3e37f900dd.tar.gz} (100%)
 rename cypress/fixtures/{1c480a4573d2a003fc2630c21c2b25829de49972.gitfast.gz => swh:1:rev:1c480a4573d2a003fc2630c21c2b25829de49972.gitfast.gz} (100%)
Changes applied before test
commit c402ae6a4ca6355519903c5c61f11fa4f81ccb7a
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Wed Aug 18 19:02:10 2021 +0200

    vault API: Rename bundle types and use SWHIDs to identify objects
    
    This is to match changes in the vault interface, see D6112 for the rationale.
    
    All existing queries still work, through "fallback" views.  There are
    some minor changes in returned fields (removed obj_type, and replace
    obj_id with swhid), but I don't think anyone used them (since that info
    is already available to them before they send the request).

commit 773d6c2e067fa6a494d2bd4e34317fff779affe8
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Aug 24 11:23:53 2021 +0200

    vault.spec.js: Add test checking the LocalStorage is used to show the task list

commit 7dde8550288892a23ea3128a52e4a67d3aad9802
Author: Valentin Lorentz <vlorentz@softwareheritage.org>
Date:   Tue Aug 24 10:19:22 2021 +0200

    vault.spec.js: Remove vaultItems from global variables
    
    So tests don't have side-effects

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

swh/web/api/views/vault.py
140

If we do not want to keep it in the index, another solution to inform about the deprecation would be to reference the deprecated URL in the new endpoint documentation (in a warning block maybe). That information should be easily discoverable from my point of view.

We could also add a separate table of deprecated endpoints in the api index page based on the use of a deprecated tag.
I would rather go for that one.

This revision is now accepted and ready to land.Aug 26 2021, 10:33 AM
swh/web/api/views/vault.py
140

(FTR, this is done in D6132)