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

Allow for passing in previously compiled grains to custom grain modules #47372

Merged
merged 3 commits into from May 29, 2018
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -269,6 +269,11 @@ grain will override that core grain. Similarly, grains from
``/etc/salt/minion`` override both core grains and custom grain modules, and
grains in ``_grains`` will override *any* grains of the same name.

For custom grains, if the function takes an argument ``grains``, then the
previously rendered grains will be passed in. Because the rest of the grains
could be rendered in any order, the only grains that can be relied upon to be
passed in are ``core`` grains. This was added in the Fluorine Release.

This comment has been minimized.

Copy link
@isbm

isbm Apr 28, 2018

Contributor

Typo. "Passed in a core grains", I believe.

This comment has been minimized.

Copy link
@gtmanfred

gtmanfred Apr 28, 2018

Author Contributor

that is not correct.

This comment has been minimized.

Copy link
@cachedout

cachedout May 8, 2018

Collaborator

Release should not be capitalized though. ;)



Examples of Grains
==================
@@ -5,6 +5,16 @@ Salt Release Notes - Codename Fluorine
======================================


Grains Dictionary Passed into Custom Grains
-------------------------------------------

Starting in this release, if a custom grains function accepts a variable named
``grains``, the Grains dictionary of the already compiled grains will be passed
in. Because of the non deterministic order that grains are rendered in, the

This comment has been minimized.

Copy link
@cachedout

cachedout May 8, 2018

Collaborator

non-deterministic

only grains that can be relied upon to be passed in are ``core.py`` grains,
since those are compiled first.


"Virtual Package" Support Dropped for APT
-----------------------------------------

@@ -751,8 +751,14 @@ def grains(opts, force_refresh=False, proxy=None):
# proxymodule for retrieving information from the connected
# device.
log.trace('Loading %s grain', key)
if funcs[key].__code__.co_argcount == 1:
ret = funcs[key](proxy)
parameters = list(funcs[key].__code__.co_varnames)
if funcs[key].__code__.co_argcount > 0:

This comment has been minimized.

Copy link
@isbm

isbm Apr 28, 2018

Contributor

The > 0 is not really needed. :-) Enough here if parameters:. But the whole check is also not needed (see below).

kwargs = {}
if 'proxy' in parameters:
kwargs['proxy'] = proxy
if 'grains' in parameters:
kwargs['grains'] = grains_data
ret = funcs[key](**kwargs)

This comment has been minimized.

Copy link
@isbm

isbm Apr 28, 2018

Contributor

No need to double-check length of the co_varnames as well as no need to if/else to call funcs[key] with or without kwargs. If parameters is an empty list, kwargs stays that way too, and so the call funcs[key](**{}) equals to funcs[key]() as so:

parameters = list(funcs[key].__code__.co_varnames)
kwargs = {}
if 'proxy' in parameters:
    kwargs['proxy'] = proxy
if 'grains' in parameters:
    kwargs['grains'] = grains_data
ret = funcs[key](**kwargs)

This comment has been minimized.

Copy link
@gtmanfred

gtmanfred Apr 28, 2018

Author Contributor

good point, and it looks like you can pass an empty kwargs to a function that doesn't take any arguments, TIL

else:
ret = funcs[key]()
except Exception:
@@ -0,0 +1,4 @@
def test(grains):
if 'os' in grains:
return {'custom_grain_test': 'itworked'}
return {'custom_grain_test': 'itdidntwork'}
@@ -0,0 +1,22 @@
# -*- coding: utf-8 -*-
'''
Test the core grains
'''

# Import python libs
from __future__ import absolute_import, unicode_literals

# Import Salt Testing libs
from tests.support.case import ModuleCase


class TestGrainsCore(ModuleCase):
'''
Test the core grains grains
'''

def test_grains_passed_to_custom_grain(self):
'''
test if current grains are passed to grains module functions that have a grains argument
'''
self.assertEqual(self.run_function('grains.get', ['custom_grain_test']), 'itworked')

This comment has been minimized.

Copy link
@isbm

isbm Apr 28, 2018

Contributor

'cmon, Dani, use PyTest, let's drop this camel case finally! 😉 Just assert that.

This comment has been minimized.

Copy link
@gtmanfred

gtmanfred Apr 28, 2018

Author Contributor

We are still using Unittests, so I am going to stick to unittests asserts until that changes.

This comment has been minimized.

Copy link
@isbm

isbm Apr 30, 2018

Contributor

Actually, PyTest is now the next thing and we already running it all over the place anyway. Why would one invest in soon-to-be-dead old stuff and thus create another technical debt that will cause rewrite in the near distant future? I wouldn't do that.

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.