Page MenuHomeSoftware Heritage
Differential D5569

Populate the visit status value in save code now request
ClosedPublic
Actions

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

Details

Reviewers
anlambert
Group Reviewers
Reviewers
Maniphest Tasks
T3266: Improve save code now failed/uneventful status reporting
Commits
rDWAPPS83f93270101e: Populate the visit status value in save code now request
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

Event Timeline

ardumont created this revision.Apr 21 2021, 2:02 PM
ardumont edited the test plan for this revision. (Show Details)Apr 21 2021, 2:07 PM
ardumont edited the test plan for this revision. (Show Details)
ardumont updated this revision to Diff 19900.Apr 21 2021, 2:08 PM

I stashed one thing too much

swh-public-ci added a comment.Apr 21 2021, 2:10 PM

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.

Harbormaster completed remote builds in B20905: Diff 19899.Apr 21 2021, 2:10 PM
ardumont added inline comments.Apr 21 2021, 2:12 PM
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

swh-public-ci added a comment.Apr 21 2021, 2:17 PM

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.

Harbormaster completed remote builds in B20906: Diff 19900.Apr 21 2021, 2:17 PM
ardumont requested review of this revision.Apr 21 2021, 2:17 PM
anlambert added a subscriber: anlambert.Apr 21 2021, 2:26 PM

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.

ardumont added a comment.Apr 21 2021, 2:37 PM

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.

anlambert requested changes to this revision.Apr 21 2021, 3:49 PM

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
anlambert added inline comments.Apr 21 2021, 3:50 PM
swh/web/common/origin_save.py
304

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

ardumont added a comment.Apr 21 2021, 3:53 PM

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?

anlambert added a comment.Apr 21 2021, 4:23 PM
In D5569#141327, @ardumont wrote:

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.

ardumont added inline comments.Apr 21 2021, 5:13 PM
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 ;)

ardumont updated this revision to Diff 19908.Apr 21 2021, 5:30 PM

Update endpoint docstring (still remains to add test)

anlambert added inline comments.Apr 21 2021, 5:37 PM
swh/web/api/views/origin_save.py
77

The visit_date field doc is also missing.

swh-public-ci added a comment.Apr 21 2021, 5:37 PM

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.

Harbormaster completed remote builds in B20913: Diff 19908.Apr 21 2021, 5:37 PM
ardumont added inline comments.Apr 21 2021, 5:42 PM
swh/web/api/views/origin_save.py
77

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?

anlambert added inline comments.Apr 21 2021, 5:46 PM
swh/web/api/views/origin_save.py
77

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

ardumont added inline comments.Apr 21 2021, 5:54 PM
swh/web/api/views/origin_save.py
77

yep, done, thx.

Now on to finding and updating the missing tests.

ardumont marked 5 inline comments as done.Apr 21 2021, 5:54 PM
ardumont updated this revision to Diff 19911.Apr 21 2021, 6:08 PM

Update existing tests

swh-public-ci added a comment.Apr 21 2021, 6:15 PM

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

Harbormaster failed remote builds in B20916: Diff 19911!Apr 21 2021, 6:15 PM
anlambert added a comment.Apr 21 2021, 6:18 PM
In D5569#141395, @swh-public-ci wrote:

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 ...

swh-public-ci added a comment.Apr 21 2021, 6:36 PM

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.

Harbormaster completed remote builds in B20916: Diff 19911.Apr 21 2021, 6:36 PM
anlambert accepted this revision.Apr 21 2021, 6:37 PM

Looks good to me.

This revision is now accepted and ready to land.Apr 21 2021, 6:37 PM
ardumont updated this revision to Diff 19913.Apr 21 2021, 6:39 PM

Rebase

ardumont added a child revision: D5570: Display visit status information in the save request information.Apr 21 2021, 6:40 PM
swh-public-ci added a comment.Apr 21 2021, 6:53 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.

Harbormaster completed remote builds in B20918: Diff 19913.Apr 21 2021, 6:53 PM
Closed by commit rDWAPPS83f93270101e: Populate the visit status value in save code now request (authored by ardumont). · Explain WhyApr 21 2021, 7:07 PM
This revision was automatically updated to reflect the committed changes.
ardumont added a commit: rDWAPPS83f93270101e: Populate the visit status value in save code now request.

Revision Contents
Changeset List

PathSizeCoverage (All)Coverage (Touched)
swh/
web/
api/
views/
6 lines100%
Loading...
common/
8 lines93%
Loading...
tests/
api/
views/
11 lines100%
Loading...

Diff 19911

View Options

swh/web/api/views/origin_save.py

Loading...
View Options

swh/web/common/origin_save.py

Loading...
View Options

swh/web/tests/api/views/test_origin_save.py

Loading...
About · Developer information · Legal