Page MenuHomeSoftware Heritage

lister/tasks: Standardize return statements

Authored by ardumont on Dec 2 2019, 4:01 PM.



The following commit adapts the return statements from both lister and their
associated tasks. This standardizes on what other modules (e.g. both dvcs and
package loaders) do.

It's apparently unnecessary as the listener seems to do what's necessary
already [1] (but it misses tests).

One could argue that having workers (lister, loader, cooker)'s code aligned
would be better in that regard (Note that we could also go the other way

Well, as i started this, might as well propose it...


Test Plan


Diff Detail

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

Event Timeline

Not a requirement/blocker at all—but while you're at this it would be nice to also add type annotations to capture the fact that these method return something else than None.
That way if the return statements get dropped by mistake in the future, mypy will complain.

Looks good to me.

As for @zack's comment, I think the most useful type annotation would be on the shared_task decorator, rather than annotating individual functions. This way we can type check:

  • the arguments of functions that end up being tasks, e.g. to force the existence of a given named argument such as url for loading tasks.
  • the return value of the task (which would be a dict, probably

We could even do specialized decorators depending on the expected shape of the arguments between listing and loading.

I don't know if there's much value adding type annotations to the task functions themselves, as they'll not really be enforced?

This revision is now accepted and ready to land.Dec 3 2019, 11:40 AM
ardumont retitled this revision from lister/tasks: Standardize return statements? to lister/tasks: Standardize return statements.Dec 4 2019, 3:16 PM