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

utils/schedule.py:handle_func() - Fix for accessing returner configur… #33912

Merged

Conversation

abalashov
Copy link
Contributor

What does this PR do?

Fixes incorrect data member names accessed for returner configuration in the schedule module.

What issues does this PR fix or reference?

Issue #33868.

Previous Behavior

Remove this section if not relevant

New Behavior

See #33868.

Tests written?

No.

…ation attributes 'return_config' and 'return_kwargs'.

They are specified as 'return_config' and 'return_kwargs' in the documentation, and those are the member names used elsewhere in the code base. However, this code previously referenced 'returner_kwargs' and 'returner_config', so returners from scheduled jobs were not appropriately called in situations in which configuration parameters to override default minion config needed to be provided to the schedule module.

Resolves issue #33868 (#33868).

…ation attributes 'return_config' and 'return_kwargs'.

They are specified as 'return_config' and 'return_kwargs' in the documentation, and those are the member names used elsewhere in the code base. However, this code previously referenced 'returner_kwargs' and 'returner_config', so returners from scheduled jobs were not appropriately called in situations in which configuration parameters to override default minion config needed to be provided to the schedule module.

Resolves issue saltstack#33868 (saltstack#33868).
@cachedout
Copy link
Contributor

Thanks, @abalashov

@garethgreenaway is this something that you'd like to take a look at?

@cachedout cachedout added the Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged label Jun 10, 2016
@garethgreenaway
Copy link
Contributor

@cachedout @abalashov Fix looks right to me. Would be good to include that item in the documentation for the schedule execution module as well.

@abalashov
Copy link
Contributor Author

abalashov commented Jun 10, 2016

The documentation is already correct on this point; the code just didn't line up with it.

That said, the schedule execution module has literally no documentation for the parameters that can be passed into schedule.add. As a newbie, it was confusing to have to infer that these parameters can be passed into execution. from reading the documentation for the schedule state module.

-- Alex

Principal, Evariste Systems LLC (www.evaristesys.com)

Sent from my Google Nexus.

@abalashov
Copy link
Contributor Author

I would be happy to try to tackle this on my own, but don't know how the docs are generated or where to make the appropriate modifications. I get the impression it's all in YAML blocks at the top of the source files. Are there any guides for this? Sorry for the newbie question.

@cachedout
Copy link
Contributor

cachedout commented Jun 13, 2016

Hi @abalashov . Function docs are pulled out of function doc strings but more in-depth documentation is all here: https://github.com/saltstack/salt/tree/develop/doc

I'm going to merge this for now but a follow-up PR with additional docs would be very welcome!

@cachedout cachedout merged commit 8d8ed59 into saltstack:2016.3 Jun 13, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Pending-Discussion The issue or pull request needs more discussion before it can be closed or merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants