core/lister: Make the tasks take an explicit lister_args argument
ClosedPublic
Actions

Authored by ardumont on Jul 17 2018, 2:09 PM.

Details

Reviewers

olasd

Group Reviewers

Reviewers

Commits

rDLSd88f1b60c908: core/lister: Make the tasks take an explicit lister_args argument

Summary

Avoid eating *all* arbitrary arguments and passing them along to the
new_lister method.

Diff Detail

Repository

rDLS Listers

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ardumont created this revision.Jul 17 2018, 2:09 PM

Herald added a reviewer: Reviewers. · View Herald TranscriptJul 17 2018, 2:09 PM

Harbormaster completed remote builds in B1346: Diff 1206.Jul 17 2018, 2:15 PM

ardumont added inline comments.Jul 17 2018, 2:20 PM

swh/lister/gitlab/tasks.py
37	carnage!

Remove spurious print statement

Harbormaster completed remote builds in B1347: Diff 1222.Jul 17 2018, 2:22 PM

ardumont marked an inline comment as done.Jul 17 2018, 2:24 PM

ardumont added inline comments.

swh/lister/gitlab/tasks.py
37	spurious print statement removed!

Please don't use mutable values (e.g. {}) as default argument, as they can get mutated and that gives surprising results.

Could you make lister_args a keyword-only argument (i.e. def run_task(self, *, lister_args=None):, and then be explicit about the argument name in the call sites? That way there's no chance the lister_args argument could be mistaken for something else.

In D392#7459, @olasd wrote:

Please don't use mutable values (e.g. {}) as default argument, as they can get mutated and that gives surprising results.

(FTAOD, it's not a problem in this specific instance, but it's a pattern we should generally avoid)