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

Added version argument to @depends decorator to enforce a minimum ver... #53658

Closed

Conversation

github-abcde
Copy link
Contributor

...sion requirement.

What does this PR do?

It adds a version kwarg to the @Depends decorator, which can handle any value that is able to be parsed with salt.utils.version.LooseVersion. The value of this version kwarg will be compared to the __version__-attribute of the module mentioned in the decorator (if it cannot be found (because it is a command or the module does not have the attribute), it will silently default to None). If the version is not equal or larger than the determined version (noting that when the latter is None, everything will be either equal (also None) or greater (any value)), the specific function will be removed by enforce_dependencies.

What issues does this PR fix or reference?

None that I know of

Previous Behavior

The @Depends decorator can only specify whether or not a module needs to be loaded.

New Behavior

The @Depends decorator can also specify a specific minimum version for the module that needs to be loaded.

Tests written?

No, there are tests in tests/integration/modules/test_decorators.py, but I have no idea how those work.

Commits signed with GPG?

Yes

@max-arnold
Copy link
Contributor

Nice! Could this be merged to Neon?

@github-abcde
Copy link
Contributor Author

github-abcde commented Jun 29, 2019

Hmm, apparently somewhere, a unicode character (the triple-dots) was introduced into the commit description, and that is causing several tests to fail (at least that is what it looks like to me). Is this something I can fix on my end (and how)?

Fixed logic path when version is insufficient.
Added version parameter in unloading log message if used.
@github-abcde github-abcde changed the title Added version argument to @depends decorator to enforce a minimum ver… Added version argument to @depends decorator to enforce a minimum ver... Jul 1, 2019
@github-abcde
Copy link
Contributor Author

Ok, that seems to have fixed the failing builds.
@max-arnold What do I need to do to merge this to Neon?

@max-arnold
Copy link
Contributor

Ask someone from SaltStack if that is possible, and if they agree then rebase against neon branch.

@github-abcde github-abcde requested a review from a team as a code owner July 29, 2019 07:40
@github-abcde
Copy link
Contributor Author

Closed in favor of #55590

@github-abcde github-abcde deleted the depends_decorator-version branch December 10, 2019 16:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants