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

Branch

save-code-now-visit-types-check

Lint

No Linters Available

Unit

No Unit Test Coverage

Build Status

Buildable 23311
Build 36374: Phabricator diff pipeline on jenkins	Jenkins console · Jenkins
Build 36373: arc lint + arc unit

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

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!

Fix cypress tests

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.

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

Wouldn't a config option be simpler?

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

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 ;)

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.

Handle @ardumont comments

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.