Page MenuHomeSoftware Heritage
Differential D6170

origin_save: Filter out visit type with no scheduler load-* task type
ClosedPublic
Actions

Authored by anlambert on Sep 2 2021, 11:39 AM.

Details

Reviewers
vlorentz
ardumont
Group Reviewers
Reviewers
Commits
rDWAPPS9ef590cb2840: origin_save: Filter out visit type with no scheduler load-* task type
Summary

In order to ease the integration of new visit types for the "Save code now"
service, filter out those with no associated task type in scheduler db.

It means that a new visit type can be added to "Save code now" implementation
but it will not be available until the associated scheduler load task type
gets created. It enables to test the new visit type in dev environments
and deploy safely to production if the associated scheduler load task type
did not get created yet.

Also remove the internal endpoint to get savable visit types, those can
be simply added in Django template.

As that change broke a lot of tests due to scheduler being previously
mocked, most of the tests related to "Save code now" got improved by
removing the mocks and using the swh_scheduler pytest fixture.

Related to D5992

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

anlambert created this revision.Sep 2 2021, 11:39 AM
Herald added a reviewer: Reviewers. · View Herald TranscriptSep 2 2021, 11:39 AM
anlambert mentioned this in D5992: add support for the CVS loader to 'Save Code Now'.Sep 2 2021, 11:42 AM
swh-public-ci added a comment.Sep 2 2021, 11:53 AM

Build has FAILED

Patch application report for D6170 (id=22353)

Rebasing onto f15e17c406...

Current branch diff-target is up to date.
Changes applied before test
commit 177cefc1b2a78f51381b69f6aed1a0a032eceff6
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu Sep 2 11:19:31 2021 +0200

    origin_save: Filter out visit type with no scheduler load-* task type
    
    In order to ease the integration of new visit types for the "Save code now"
    service, filter out those with no associated task type in scheduler db.
    
    It means that a new visit type can be added to "Save code now" implementation
    but it will not be available until the associated scheduler load task type
    gets created. It enables to test the new visit type in dev environments
    and deploy safely to production if the associated scheduler load task type
    did not get created yet.
    
    Also remove the internal endpoint to get savable visit types, those can
    be simply added in Django template.
    
    As that change broke a lot of tests due to scheduler being previously
    mocked, most of the tests related to "Save code now" got improved by
    removing the mocks and using the swh_scheduler pytest fixture.
    
    Related to D5992

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

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 2 2021, 11:53 AM
Harbormaster failed remote builds in B23309: Diff 22353!
anlambert updated this revision to Diff 22354.Sep 2 2021, 12:07 PM

Fix cypress tests

swh-public-ci added a comment.Sep 2 2021, 12:24 PM

Build has FAILED

Patch application report for D6170 (id=22354)

Rebasing onto f15e17c406...

Current branch diff-target is up to date.
Changes applied before test
commit 461c2ba1706f37817594931fc6274744407e041b
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu Sep 2 11:19:31 2021 +0200

    origin_save: Filter out visit type with no scheduler load-* task type
    
    In order to ease the integration of new visit types for the "Save code now"
    service, filter out those with no associated task type in scheduler db.
    
    It means that a new visit type can be added to "Save code now" implementation
    but it will not be available until the associated scheduler load task type
    gets created. It enables to test the new visit type in dev environments
    and deploy safely to production if the associated scheduler load task type
    did not get created yet.
    
    Also remove the internal endpoint to get savable visit types, those can
    be simply added in Django template.
    
    As that change broke a lot of tests due to scheduler being previously
    mocked, most of the tests related to "Save code now" got improved by
    removing the mocks and using the swh_scheduler pytest fixture.
    
    Related to D5992

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

Harbormaster returned this revision to the author for changes because remote builds failed.Sep 2 2021, 12:24 PM
Harbormaster failed remote builds in B23310: Diff 22354!
swh-public-ci added a comment.Sep 2 2021, 1:21 PM

Build is green

Patch application report for D6170 (id=22354)

Rebasing onto f15e17c406...

Current branch diff-target is up to date.
Changes applied before test
commit 461c2ba1706f37817594931fc6274744407e041b
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu Sep 2 11:19:31 2021 +0200

    origin_save: Filter out visit type with no scheduler load-* task type
    
    In order to ease the integration of new visit types for the "Save code now"
    service, filter out those with no associated task type in scheduler db.
    
    It means that a new visit type can be added to "Save code now" implementation
    but it will not be available until the associated scheduler load task type
    gets created. It enables to test the new visit type in dev environments
    and deploy safely to production if the associated scheduler load task type
    did not get created yet.
    
    Also remove the internal endpoint to get savable visit types, those can
    be simply added in Django template.
    
    As that change broke a lot of tests due to scheduler being previously
    mocked, most of the tests related to "Save code now" got improved by
    removing the mocks and using the swh_scheduler pytest fixture.
    
    Related to D5992

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

Harbormaster completed remote builds in B23310: Diff 22354.Sep 2 2021, 1:21 PM
anlambert requested review of this revision.Sep 2 2021, 1:21 PM
vlorentz added a subscriber: vlorentz.Sep 2 2021, 1:26 PM

Wouldn't a config option be simpler?

anlambert added a subscriber: ardumont.Sep 2 2021, 1:47 PM
In D6170#159680, @vlorentz wrote:

Wouldn't a config option be simpler?

We thought about that first with @ardumont (D5992#159569) but this would require
touching production and docker environment configuration for swh-web each time a
new visit type is added which is not really convenient.

If the load task type is present in scheduler database, we are sure that tasks can be
created so that approach.

vlorentz accepted this revision.Sep 2 2021, 2:04 PM
This revision is now accepted and ready to land.Sep 2 2021, 2:04 PM
ardumont accepted this revision.EditedSep 2 2021, 2:07 PM

Thanks.

I like how you were able to scrub out the scheduler mocking part as well, nice work!
That had become a mess to maintain so kudos.

swh/web/tests/conftest.py
449

not that it matters here but it's svn ;)

466

same ;)

anlambert added a comment.Sep 2 2021, 2:14 PM
In D6170#159685, @ardumont wrote:

Thanks.

I like how you were able to scrub out the scheduler mocking part as well, nice work!
That had become a mess to maintain so kudos.

Yes, it was about time to remove those mocks.
Tests should be simpler to maintain and we should catch any breaking changes related to the scheduler this way.
Thanks for notifying wrong module paths, I will fix those.

anlambert updated this revision to Diff 22357.Sep 2 2021, 2:23 PM

Handle @ardumont comments

swh-public-ci added a comment.Sep 2 2021, 2:37 PM

Build is green

Patch application report for D6170 (id=22357)

Rebasing onto f15e17c406...

Current branch diff-target is up to date.
Changes applied before test
commit 9ef590cb28400765163293d56d1d844de99f23d6
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu Sep 2 11:19:31 2021 +0200

    origin_save: Filter out visit type with no scheduler load-* task type
    
    In order to ease the integration of new visit types for the "Save code now"
    service, filter out those with no associated task type in scheduler db.
    
    It means that a new visit type can be added to "Save code now" implementation
    but it will not be available until the associated scheduler load task type
    gets created. It enables to test the new visit type in dev environments
    and deploy safely to production if the associated scheduler load task type
    did not get created yet.
    
    Also remove the internal endpoint to get savable visit types, those can
    be simply added in Django template.
    
    As that change broke a lot of tests due to scheduler being previously
    mocked, most of the tests related to "Save code now" got improved by
    removing the mocks and using the swh_scheduler pytest fixture.
    
    Related to D5992

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

Harbormaster completed remote builds in B23311: Diff 22357.Sep 2 2021, 2:37 PM
Closed by commit rDWAPPS9ef590cb2840: origin_save: Filter out visit type with no scheduler load-* task type (authored by anlambert). · Explain WhySep 2 2021, 2:40 PM
This revision was automatically updated to reflect the committed changes.
anlambert added a commit: rDWAPPS9ef590cb2840: origin_save: Filter out visit type with no scheduler load-* task type.

Revision Contents
Changeset List

PathSize
assets/
src/
bundles/
save/
8 lines
swh/
web/
common/
34 lines
misc/
14 lines
templates/
misc/
4 lines
tests/
admin/
38 lines
api/
views/
238 lines
browse/
views/
8 lines
4 lines
common/
147 lines
51 lines
misc/
2 lines
27 lines

Diff 22358

View Options

assets/src/bundles/save/index.js

Loading...
View Options

swh/web/common/origin_save.py

Loading...
View Options

swh/web/misc/origin_save.py

Loading...
View Options

swh/web/templates/misc/origin-save.html

Loading...
View Options

swh/web/tests/admin/test_origin_save.py

Loading...
View Options

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

Loading...
View Options

swh/web/tests/browse/views/test_origin.py

Loading...
View Options

swh/web/tests/browse/views/test_revision.py

Loading...
View Options

swh/web/tests/common/test_origin_save.py

Loading...
View Options

swh/web/tests/conftest.py

Loading...
View Options

swh/web/tests/misc/test_metrics.py

Loading...
View Options

swh/web/tests/misc/test_origin_save.py

Loading...
About · Developer information · Legal