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

State module.run/wait misses args when looking for kwargs #42270

Closed
wants to merge 1 commit into from
Closed

State module.run/wait misses args when looking for kwargs #42270

wants to merge 1 commit into from

Conversation

The-Loeki
Copy link
Contributor

What does this PR do?

The new syntax parser for the module.run/wait fails to account for args set, and incorrectly reports missing arguments as a result.
This PR aims to fix that.

ima.py:

def teapot(arg1, arg2, **kwargs):
  pass

just a state:

just-an-ex:
  module.run:
    - ima.teapot: 
      - bla
      - example: bla

res:

[ERROR   ] 'ima.teapot' failed: Missing arguments: arg1

What issues does this PR fix or reference?

#42148

@ghost
Copy link

ghost commented Jul 12, 2017

@The-Loeki, thanks for your PR! By analyzing the history of the files in this pull request, we identified @isbm, @thatch45 and @terminalmage to be potential reviewers.

@isbm
Copy link
Contributor

isbm commented Jul 12, 2017

This, in fact, doesn't solve the problem. Apparently I found that:

def foo(a, b, c, *args, **kwargs):

...would be a bit problematic, since the state is checking if there is an arguments for a, b and c, but clashing with the args. I am taking care of it right now.

And this needs a unit test. So please close this PR, I will only refer to this one in mine. Thanks!

@The-Loeki
Copy link
Contributor Author

I was afraid of something like that but hadn't had chance to test yet.

Tnx & closing :)

@The-Loeki The-Loeki closed this Jul 12, 2017
@The-Loeki The-Loeki deleted the state-module-args-n-kwargs branch July 12, 2017 09:35
@isbm
Copy link
Contributor

isbm commented Jul 12, 2017

Particular fault of mine that I just reused older existing code from the previous deprecated function. Good catch, I found how to properly fix it. Lesson learned: never trust a codesmell.

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