interface: Return enabled origins only by default in get_listed_origins
ClosedPublic
Actions

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

Details

Reviewers

ardumont
vlorentz

Group Reviewers

Reviewers

Commits

rDSCHe56fc4d1edca: interface: Return enabled origins only by default in get_listed_origins

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

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.May 12 2022, 11:26 AM

Herald added a reviewer: Reviewers. · View Herald TranscriptMay 12 2022, 11:26 AM

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.

Harbormaster completed remote builds in B29334: Diff 28227.May 12 2022, 11:31 AM

anlambert requested review of this revision.May 12 2022, 11:31 AM

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.

In D7816#203364, @vlorentz wrote:

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.

Address @vlorentz comment

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.

Harbormaster completed remote builds in B29336: Diff 28229.May 12 2022, 12:12 PM