Page MenuHomeSoftware Heritage

Populate the visit status value in save code now request
ClosedPublic

Authored by ardumont on Apr 21 2021, 2:02 PM.

Details

Summary

Related to T3266
Depends on D5555

Test Plan

tox

happy tests are still happy

container: db row after a save code now have their visit_status populated

swh-web=# select * from save_origin_request order by request_date desc;
 id |         request_date          | visit_type |                           origin_url                           |  status  | loading_task_id |          visit_date           | loading_task_status | visit_status
----+-------------------------------+------------+----------------------------------------------------------------+----------+-----------------+-------------------------------+---------------------+--------------
  9 | 2021-04-21 12:03:52.083365+00 | git        | https://github.com/WerWolv/ImHex                               | accepted |              18 |                               | running             |
  8 | 2021-04-21 11:57:40.967667+00 | git        | https://github.com/divnix/devos                                | accepted |              16 | 2021-04-21 11:58:34.487761+00 | succeeded           | full
  7 | 2021-04-21 11:32:02.161929+00 | git        | https://github.com/org-trello/org-trello                       | accepted |              14 | 2021-04-21 11:34:02.994894+00 | succeeded           | full

Diff Detail

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

Event Timeline

ardumont edited the test plan for this revision. (Show Details)

I stashed one thing too much

Build is green

Patch application report for D5569 (id=19899)

Could not rebase; Attempt merge onto 015ddb8964...

Merge made by the 'recursive' strategy.
 requirements-test.txt                              |  1 +
 .../0009_saveoriginrequest_visit_status.py         | 32 ++++++++++++++++++++++
 swh/web/common/models.py                           | 18 ++++++++++++
 swh/web/common/origin_save.py                      |  7 +++--
 swh/web/tests/common/test_origin_save.py           |  6 ++++
 swh/web/tests/test_migrations.py                   | 23 ++++++++++++++++
 6 files changed, 85 insertions(+), 2 deletions(-)
 create mode 100644 swh/web/common/migrations/0009_saveoriginrequest_visit_status.py
 create mode 100644 swh/web/tests/test_migrations.py
Changes applied before test
commit 0417f2a605105398f9c5ca300779fee5302e3ae3
Merge: 015ddb89 c442ab59
Author: Jenkins user <jenkins@localhost>
Date:   Wed Apr 21 12:02:58 2021 +0000

    Merge branch 'diff-target' into HEAD

commit c442ab5912335ea43059a23a5b70094934445cb0
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 21 13:59:19 2021 +0200

    Populate the visit status value in save code now request
    
    Related to T3266

commit fb1f5bbd85137f0754435c10784d03aa41efd20e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Apr 19 18:26:33 2021 +0200

    Add visit_status to SaveOriginRequest model
    
    Related to T3266

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

swh/web/common/origin_save.py
222

I think that possibly happens due to missing updating the tasks in the save code now ui [1].
Which should be attended when [2] is fixed (soon).

(or something)

[1] T3278#63827

[2] T3280

Build is green

Patch application report for D5569 (id=19900)

Could not rebase; Attempt merge onto 015ddb8964...

Merge made by the 'recursive' strategy.
 requirements-test.txt                              |  1 +
 .../0009_saveoriginrequest_visit_status.py         | 32 ++++++++++++++++++++++
 swh/web/common/models.py                           | 18 ++++++++++++
 swh/web/common/origin_save.py                      |  9 ++++--
 swh/web/tests/common/test_origin_save.py           |  6 ++++
 swh/web/tests/test_migrations.py                   | 23 ++++++++++++++++
 6 files changed, 86 insertions(+), 3 deletions(-)
 create mode 100644 swh/web/common/migrations/0009_saveoriginrequest_visit_status.py
 create mode 100644 swh/web/tests/test_migrations.py
Changes applied before test
commit 8ebfd2599690c5fa6fe1d1a214ea576f346b0fb4
Merge: 015ddb89 1e7e00b3
Author: Jenkins user <jenkins@localhost>
Date:   Wed Apr 21 12:10:34 2021 +0000

    Merge branch 'diff-target' into HEAD

commit 1e7e00b3cab0749ed72d7a6800594beb1b6f4ee5
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 21 13:59:19 2021 +0200

    Populate the visit status value in save code now request
    
    Related to T3266

commit fb1f5bbd85137f0754435c10784d03aa41efd20e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Apr 19 18:26:33 2021 +0200

    Add visit_status to SaveOriginRequest model
    
    Related to T3266

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

Cool, you should update the tests checking that newly added value in the model.

swh/web/common/origin_save.py
222

IIRC we had some service downtime a couple of years ago due to a hardware issue. and all save requests tasks in the queue got lost.
That code was added to fix the statuses of that lost tasks and should be removed in the future.

Cool, you should update the tests checking that newly added value in the model.

yes, i'll look (although i thought the existing tests already checked).
In any case, I opened it early so the coverage could be seen here ;)

will check.

As explained in an inline comment, you should add the visit_status field in the dict associated to a request.

This will allow to have that status returned in Web API responses.

Endpoint documentation should also be updated with that newly added field.

swh/web/common/origin_save.py
304

You should also add the visit_status in that dict.

This revision now requires changes to proceed.Apr 21 2021, 3:49 PM
swh/web/common/origin_save.py
304

forget that comment, I did not see it was already there ...

So there is actually just trying to add some more coverage (trying to find where it's supposed to be) and the actual documentation endpoint to update, right?

So there is actually just trying to add some more coverage (trying to find where it's supposed to be) and the actual documentation endpoint to update, right?

Sounds good, yes.

swh/web/common/origin_save.py
222

Yeah, i recall something like that now that you refreshed me a bit on this.

Note that I don't think that excludes what i said nonetheless ;)

Update endpoint docstring (still remains to add test)

swh/web/api/views/origin_save.py
74 ↗(On Diff #19908)

The visit_date field doc is also missing.

Build is green

Patch application report for D5569 (id=19908)

Could not rebase; Attempt merge onto 015ddb8964...

Merge made by the 'recursive' strategy.
 requirements-test.txt                              |  1 +
 swh/web/api/views/origin_save.py                   |  2 ++
 .../0009_saveoriginrequest_visit_status.py         | 32 ++++++++++++++++++++++
 swh/web/common/models.py                           | 18 ++++++++++++
 swh/web/common/origin_save.py                      |  9 ++++--
 swh/web/tests/common/test_origin_save.py           |  6 ++++
 swh/web/tests/test_migrations.py                   | 23 ++++++++++++++++
 7 files changed, 88 insertions(+), 3 deletions(-)
 create mode 100644 swh/web/common/migrations/0009_saveoriginrequest_visit_status.py
 create mode 100644 swh/web/tests/test_migrations.py
Changes applied before test
commit d62d880c927b43e68a9bfad6beb1421c0eac5ca3
Merge: 015ddb89 80f35d1e
Author: Jenkins user <jenkins@localhost>
Date:   Wed Apr 21 15:31:06 2021 +0000

    Merge branch 'diff-target' into HEAD

commit 80f35d1ef288a8f8dad8edcf61e2bdebe839c504
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 21 13:59:19 2021 +0200

    Populate the visit status value in save code now request
    
    Related to T3266

commit fb1f5bbd85137f0754435c10784d03aa41efd20e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Apr 19 18:26:33 2021 +0200

    Add visit_status to SaveOriginRequest model
    
    Related to T3266

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

swh/web/api/views/origin_save.py
74 ↗(On Diff #19908)

I'll add it.

For the null entry what's the correct syntax btw?
It's not a raw null as text we will have but a null value. Is there a way to distinguish?

swh/web/api/views/origin_save.py
74 ↗(On Diff #19908)

You can say that a null value will be returned when no visit has been made so far.

swh/web/api/views/origin_save.py
74 ↗(On Diff #19908)

yep, done, thx.

Now on to finding and updating the missing tests.

Build has FAILED

Patch application report for D5569 (id=19911)

Could not rebase; Attempt merge onto 015ddb8964...

Merge made by the 'recursive' strategy.
 requirements-test.txt                              |  1 +
 swh/web/api/views/origin_save.py                   |  6 ++++
 .../0009_saveoriginrequest_visit_status.py         | 32 ++++++++++++++++++++++
 swh/web/common/models.py                           | 18 ++++++++++++
 swh/web/common/origin_save.py                      |  9 ++++--
 swh/web/tests/api/views/test_origin_save.py        | 11 ++++++--
 swh/web/tests/common/test_origin_save.py           |  6 ++++
 swh/web/tests/test_migrations.py                   | 23 ++++++++++++++++
 8 files changed, 101 insertions(+), 5 deletions(-)
 create mode 100644 swh/web/common/migrations/0009_saveoriginrequest_visit_status.py
 create mode 100644 swh/web/tests/test_migrations.py
Changes applied before test
commit 1847a2acf5ec0466a4a5598282a38328f0f196ef
Merge: 015ddb89 9cf8153b
Author: Jenkins user <jenkins@localhost>
Date:   Wed Apr 21 16:08:39 2021 +0000

    Merge branch 'diff-target' into HEAD

commit 9cf8153b43631942660e52457afb20d670fc9077
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 21 13:59:19 2021 +0200

    Populate the visit status value in save code now request
    
    Related to T3266

commit fb1f5bbd85137f0754435c10784d03aa41efd20e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Apr 19 18:26:33 2021 +0200

    Add visit_status to SaveOriginRequest model
    
    Related to T3266

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

Build has FAILED

Patch application report for D5569 (id=19911)

Could not rebase; Attempt merge onto 015ddb8964...

Merge made by the 'recursive' strategy.
 requirements-test.txt                              |  1 +
 swh/web/api/views/origin_save.py                   |  6 ++++
 .../0009_saveoriginrequest_visit_status.py         | 32 ++++++++++++++++++++++
 swh/web/common/models.py                           | 18 ++++++++++++
 swh/web/common/origin_save.py                      |  9 ++++--
 swh/web/tests/api/views/test_origin_save.py        | 11 ++++++--
 swh/web/tests/common/test_origin_save.py           |  6 ++++
 swh/web/tests/test_migrations.py                   | 23 ++++++++++++++++
 8 files changed, 101 insertions(+), 5 deletions(-)
 create mode 100644 swh/web/common/migrations/0009_saveoriginrequest_visit_status.py
 create mode 100644 swh/web/tests/test_migrations.py
Changes applied before test
commit 1847a2acf5ec0466a4a5598282a38328f0f196ef
Merge: 015ddb89 9cf8153b
Author: Jenkins user <jenkins@localhost>
Date:   Wed Apr 21 16:08:39 2021 +0000

    Merge branch 'diff-target' into HEAD

commit 9cf8153b43631942660e52457afb20d670fc9077
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 21 13:59:19 2021 +0200

    Populate the visit status value in save code now request
    
    Related to T3266

commit fb1f5bbd85137f0754435c10784d03aa41efd20e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Apr 19 18:26:33 2021 +0200

    Add visit_status to SaveOriginRequest model
    
    Related to T3266

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

Those bloody flaky cypress tests, I should hunt them when I have the time ...

Build is green

Patch application report for D5569 (id=19911)

Could not rebase; Attempt merge onto 015ddb8964...

Merge made by the 'recursive' strategy.
 requirements-test.txt                              |  1 +
 swh/web/api/views/origin_save.py                   |  6 ++++
 .../0009_saveoriginrequest_visit_status.py         | 32 ++++++++++++++++++++++
 swh/web/common/models.py                           | 18 ++++++++++++
 swh/web/common/origin_save.py                      |  9 ++++--
 swh/web/tests/api/views/test_origin_save.py        | 11 ++++++--
 swh/web/tests/common/test_origin_save.py           |  6 ++++
 swh/web/tests/test_migrations.py                   | 23 ++++++++++++++++
 8 files changed, 101 insertions(+), 5 deletions(-)
 create mode 100644 swh/web/common/migrations/0009_saveoriginrequest_visit_status.py
 create mode 100644 swh/web/tests/test_migrations.py
Changes applied before test
commit ffa0581f7a5e5b4c5b7c7e557bfc7957f7aee082
Merge: 015ddb89 9cf8153b
Author: Jenkins user <jenkins@localhost>
Date:   Wed Apr 21 16:29:40 2021 +0000

    Merge branch 'diff-target' into HEAD

commit 9cf8153b43631942660e52457afb20d670fc9077
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 21 13:59:19 2021 +0200

    Populate the visit status value in save code now request
    
    Related to T3266

commit fb1f5bbd85137f0754435c10784d03aa41efd20e
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Apr 19 18:26:33 2021 +0200

    Add visit_status to SaveOriginRequest model
    
    Related to T3266

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

This revision is now accepted and ready to land.Apr 21 2021, 6:37 PM

Build is green

Patch application report for D5569 (id=19913)

Could not rebase; Attempt merge onto 015ddb8964...

Updating 015ddb89..83f93270
Fast-forward
 requirements-test.txt                              |  1 +
 swh/web/api/views/origin_save.py                   |  6 ++++
 .../0009_saveoriginrequest_visit_status.py         | 32 ++++++++++++++++++++++
 swh/web/common/models.py                           | 18 ++++++++++++
 swh/web/common/origin_save.py                      |  9 ++++--
 swh/web/tests/api/views/test_origin_save.py        | 11 ++++++--
 swh/web/tests/common/test_origin_save.py           |  6 ++++
 swh/web/tests/test_migrations.py                   | 23 ++++++++++++++++
 8 files changed, 101 insertions(+), 5 deletions(-)
 create mode 100644 swh/web/common/migrations/0009_saveoriginrequest_visit_status.py
 create mode 100644 swh/web/tests/test_migrations.py
Changes applied before test
commit 83f93270101e2de20bf65bf829c421230b88e3fc
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Wed Apr 21 13:59:19 2021 +0200

    Populate the visit status value in save code now request
    
    Related to T3266

commit 1042f73c181d722e681a562088afcdcc2ee0af69
Author: Antoine R. Dumont (@ardumont) <ardumont@softwareheritage.org>
Date:   Mon Apr 19 18:26:33 2021 +0200

    Add visit_status to SaveOriginRequest model
    
    Related to T3266

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