Page MenuHomeSoftware Heritage

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

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

Details

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

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!

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!

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.

Wouldn't a config option be simpler?

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.

This revision is now accepted and ready to land.Sep 2 2021, 2:04 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 ;)

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.

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.