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

Difference between the onfail and onchanges requisites vs their new _any variants unclear #47015

Closed
forty8bits opened this issue Apr 11, 2018 · 9 comments
Labels
Documentation Relates to Salt documentation stale
Milestone

Comments

@forty8bits
Copy link

While the value added from the require_any and watch_any requisites introduced in v2018.3.0 is clear and useful, and the documentation explains this well, it's unclear to me how the behaviour of onfail_any and onchanges_any is supposed to differ from the standard onfail and onchanges requisites?

From the docs on onfail, confirming it uses OR logic:

Beginning in the 2016.11.0 release of Salt, onfail uses OR logic for multiple listed onfail requisites. Prior to the 2016.11.0 release, onfail used AND logic.

And for onchanges:

If a state has multiple onchanges requisites then the state will trigger if any of the watched states changes.

Perhaps onchanges would fail if any of the states listed failed, whereas onchanges_any wouldn't? It would be nice to have these ambiguities cleared up in the docs.

@gtmanfred
Copy link
Contributor

@garethgreenaway can you take a look at this to clarify the docs?

Thanks,
Daniel

@gtmanfred gtmanfred added the Documentation Relates to Salt documentation label Apr 11, 2018
@gtmanfred gtmanfred added this to the Approved milestone Apr 11, 2018
@garethgreenaway
Copy link
Contributor

Upon closer investigation it appears that onfail_any and onfail & onchanges_any and onchanges respectively perform the same function. Since onfail and onchanges existed first, we should remove the onfail_any and onchanges_any.

@mattp-
Copy link
Contributor

mattp- commented Apr 12, 2018

@garethgreenaway providing the inversion would be useful (onfail_all, onchanges_all)

@forty8bits
Copy link
Author

forty8bits commented Apr 12, 2018

I like the idea of _all and _any variants as it removes all ambiguity. I also like the idea of being able to use both variants to complement each other, for example require_all-ing a few states while also require_any-ing a few others to enable defining a combination multiple mandatory dependencies along with at-least-one-of multiple optional dependencies, although I realise that's out of the scope of this specific issue.

@mattp-
Copy link
Contributor

mattp- commented Apr 12, 2018

@forty8bits other than onfail and onchanges, every other requisite is implicitly AND, alongside _any for its OR equivalent (ie, the require_all you mentioned is already what require is), its implemented and released. we're just missing the AND for onchanges/onfail to complete the set of possible situations.
fwiw, it seems onfail originally WAS using AND, but was switched with #22370 . I'm going to take a look and see how easy it will be to revert that logic for onfail_all / onchanges_all

@forty8bits
Copy link
Author

@mattp- I understand that, I just meant migrating all requisites to be fully qualified with _all and _any across the board would be nicely consistent and unambiguous.

@mattp-
Copy link
Contributor

mattp- commented Apr 12, 2018

@forty8bits aye, agreed. i think its pretty trivial to create aliases; will try to keep that in mind when I find some tuits to look at this 👍

@stale
Copy link

stale bot commented Jul 26, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

If this issue is closed prematurely, please leave a comment and we will gladly reopen the issue.

@stale stale bot added the stale label Jul 26, 2019
@stale stale bot closed this as completed Aug 2, 2019
@baby-gnu
Copy link

baby-gnu commented Sep 4, 2019

Hello.

I think this should stay open.

Regards.

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

No branches or pull requests

5 participants