salt-ssh + grains.filter_by Type error: filter_by() got an unexpected keyword argument 'base'

[xlotlu]

Description of Issue/Question

Using salt['grains.filter_by']() over salt-ssh goes via this supposed wrapper /client/ssh/wrapper/grains.py#L141 -- which indeed has no 'base' kwarg.

One can see none of the other functions are wrappers either, but a copy-paste made some time ago that diverged from new developments.

Ironically, using salt['grains'].filter_by() or salt.grains.filter_by() does not go through the wrapper and works just fine.

Setup

example.sls

{% set test = salt['grains.filter_by']({
    'default': {'foo': 'bar'},
}, base='default') %}

test-it:
  file.managed:
    - name: /tmp/{{ test.foo }}

Steps to Reproduce Issue

Use the above snippet over salt-ssh.

Versions Report

Salt: 2016.3.0

[Ch3LL]

@xlotlu I am able to reproduce this:

ch3ll-cent7:
    - Rendering SLS 'base:issues.33911' failed: Jinja error: filter_by() got an unexpected keyword argument 'base'
      Traceback (most recent call last):
        File "/home/ch3ll/git/salt/salt/utils/templates.py", line 366, in render_jinja_tmpl
          output = template.render(**decoded_context)
        File "/usr/lib/python2.7/site-packages/jinja2/environment.py", line 969, in render
          return self.environment.handle_exception(exc_info, True)
        File "/usr/lib/python2.7/site-packages/jinja2/environment.py", line 742, in handle_exception
          reraise(exc_type, exc_value, tb)
        File "<template>", line 1, in top-level template code
      TypeError: filter_by() got an unexpected keyword argument 'base'

      ; line 1

      ---
      {% set test = salt['grains.filter_by']({    <======================
          'default': {'foo': 'bar'},
      }, base='default') %}

      test-it:
        file.managed:
      [...]
      ---

Looks like base needs to be added to that function. Thanks

[rallytime]

@xlotlu I have fixed this with #33952:

root@rallytime:~# salt-ssh -i rally-ssh state.sls example
rally-ssh:
----------
          ID: test-it
    Function: file.managed
        Name: /tmp/bar
      Result: True
     Comment: Empty file
     Started: 22:33:08.496032
    Duration: 45.968 ms
     Changes:
              ----------
              new:
                  file /tmp/bar created

Summary for rally-ssh
------------
Succeeded: 1 (changed=1)
Failed:    0
------------
Total states run:     1

[xlotlu]

@Ch3LL and @rallytime: I think ideally all functions in the wrapper should do just that, wrap the default ones, otherwise this is just asking for recurrent trouble. If you agree I'll create a new issue for a rewrite of the wrapper code (unless you want to repurpose this one), but...

Before deciding on a rewrite there's the issue of why the recommended salt['grains.filter_by']() goes through the wrapper, while salt['grains'].filter_by() doesn't -- while still working fine!
This is suggestive of some strangeness under the hood, and raises the question of why that wrapper is even needed (do note I'm new here and have no idea about why that code exists or the design decisions behind it).

I created #33966 for this so someone in the know can investigate.

[rallytime]

@xlotlu You have brought up some good points that I also had questions about when I was fixing this issue. I don't know the answers to those questions offhand, as I am not sure what the architectural decisions/concerns were when this wrapper file was created. I think for now this immediate bug is resolved and we can continue the discussion on the other issue, #33966. Thank you for opening the new issue.