Page MenuHomeSoftware Heritage

interface: Return enabled origins only by default in get_listed_origins
ClosedPublic

Authored by anlambert on May 12 2022, 11:26 AM.

Details

Summary

Add a new enabled_only parameter set to True by default in
get_listed_origins scheduler method.

It enables to filter out by default disabled listed origins
when requesting the result of a listing and avoid possible
errors in listers implementation.

Fixes SWH-LISTER-5N

Diff Detail

Repository
rDSCH Scheduling utilities
Branch
get-listed-origins-enabled-only
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 29334
Build 45837: Phabricator diff pipeline on jenkinsJenkins console · Jenkins
Build 45836: arc lint + arc unit

Event Timeline

Build is green

Patch application report for D7816 (id=28227)

Rebasing onto c7c53eab01...

Current branch diff-target is up to date.
Changes applied before test
commit deba945f47ef5db14efd953481f50eab8d24a77b
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu May 12 11:08:09 2022 +0200

    interface: Return enabled origins only by default in get_listed_origins
    
    Add a new enabled_only parameter set to True by default in
    get_listed_origins scheduler method.
    
    It enables to filter out by default disabled listed origins
    when requesting the result of a listing and avoid possible
    errors in listers implementation.

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

I think it would make more sense for the new argument to be enabled: Optional[bool].

True -> return only enabled
False -> return only disabled
None -> return both (no filtering)

It barely requires extra code (replace if enabled_only: query_filters.append("enabled = true") with if enabled is not None: query_filters.append("enabled = %s"); query_params.append(enabled)), and could be useful in the future.

I think it would make more sense for the new argument to be enabled: Optional[bool].

True -> return only enabled
False -> return only disabled
None -> return both (no filtering)

It barely requires extra code (replace if enabled_only: query_filters.append("enabled = true") with if enabled is not None: query_filters.append("enabled = %s"); query_params.append(enabled)), and could be useful in the future.

Ack, makes sense indeed, will update.

Build is green

Patch application report for D7816 (id=28229)

Rebasing onto c7c53eab01...

Current branch diff-target is up to date.
Changes applied before test
commit e56fc4d1edcaf712a60fb7884a00fe62e469ef4e
Author: Antoine Lambert <anlambert@softwareheritage.org>
Date:   Thu May 12 11:08:09 2022 +0200

    interface: Return enabled origins only by default in get_listed_origins
    
    Add a new enabled_only parameter set to True by default in
    get_listed_origins scheduler method.
    
    It enables to filter out by default disabled listed origins
    when requesting the result of a listing and avoid possible
    errors in listers implementation.

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

ardumont added a subscriber: ardumont.

Even better ;)

This revision is now accepted and ready to land.May 12 2022, 1:21 PM