Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Issue a warning when a parameter is not consumed by a task #3170

Merged
merged 1 commit into from Jun 4, 2022

Conversation

adrien-berchet
Copy link
Contributor

Description

After consuming the parameters I add a check to ensure that the task consumed all given parameters. If it is not the case a warning is raised.

Motivation and Context

In one of our workflows, several tasks have similar parameters but not all. This confused some users that tried to use such parameter with irrelevant tasks and the users were confused. Adding warnings could help users to understand what's happening. It can also help finding typos in parameter names (if they have a default value luigi does not complain and just use the default value).

Have you tested this? If so, how?

I added a unit test.

@adrien-berchet adrien-berchet requested review from dlstadther and a team as code owners June 3, 2022 16:50
Copy link
Collaborator

@dlstadther dlstadther left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Handy! Thanks for the contribution!

@dlstadther dlstadther merged commit 78145d6 into spotify:master Jun 4, 2022
@adrien-berchet
Copy link
Contributor Author

With pleasure :-)

Do you know when the next release is planned? The last one is pretty old and there are several things in the master branch we are interested in.

@dlstadther
Copy link
Collaborator

@adrien-berchet , the current release is quite old. I haven't done a Luigi release myself in years, but I can try to get one out. If so, it'll likely be 3.1.0.

Out of curiosity, are y'all currently using 3.0.3, master, or a fork?

@adrien-berchet
Copy link
Contributor Author

Ah cool, that would be great, thanks!

Currently we use the 3.0.3. But we have a repository (https://github.com/BlueBrain/luigi-tools) in which we add everything we need and then we submit PRs here for the generic features that might be interesting for others.

@dlstadther
Copy link
Collaborator

I've gotten the release started at #3171

I'll have to get a Spotify employee to approve the version bump before i can merge. At that point, i'll proceed with updating PyPi (hopefully i still have access to do so).

@adrien-berchet adrien-berchet deleted the unconsumed_param_warning branch June 7, 2022 08:02
@adrien-berchet
Copy link
Contributor Author

Ok, thank you very much!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants