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

imports in group_data files can cause "pyinfra <inventory> debug-inventory" to fail #1297

Open
david-d-white opened this issue Feb 19, 2025 · 2 comments

Comments

@david-d-white
Copy link

Describe the bug

Certain imports can cause the pyinfra <inventory> debug-inventory cli command to fail in pyinfra_cli.util.json_encode while trying to dump the inventory with pyinfra_cli.prints.jsonify()

This doesn't seem to affect anything that i've seen other than the specific command pyinfra <inventory> debug-inventory

To Reproduce

I initially stumbled across this when following the dynamic inventory example from the docs.

One way to reproduce:

  1. Add from pyinfra import inventory to any group data file used in the target inventory
  2. Run the following cli command pyinfra <inventory> debug-inventory

This produces the following error:

  File "<...>/site-packages/pyinfra_cli/util.py", line 159, in json_encode
    raise TypeError("Cannot serialize: {0} ({1})".format(type(obj), obj))
TypeError: Cannot serialize: <class 'pyinfra.context.ContextObject'> (<pyinfra.api.inventory.Inventory object at 0x7f5409b6c550>)

The same issue can be reproduced by importing any class as a type. However the error becomes the following:

  File "<...>/site-packages/pyinfra_cli/util.py", line 157, in json_encode
    return obj.to_json()
           ~~~~~~~~~~~^^
TypeError: <Class>.to_json() missing 1 required positional argument: 'self'

Expected behavior

The core issue seems to stem from pyinfra_cli treating imports as part of inventory data when parsing python files. I'm not sure what the expected behaviour should be because it depends on whether or not this is by design.

I think there are a few possibilities:

  1. pyinfra should not include imports as a part of inventory data.
  2. the command pyinfra <inventory> debug-inventory should avoid calling obj.to_json() on classes, and should have a default case for objects that don't have a to_json() function rather throwing an error.
  3. if this is intended behaviour, it should be pointed out in the docs, as well as ways to work around the behaviour when you don't want an import to be included as inventory data for this reason.

Meta

  • Include output of pyinfra --support.
    System: Linux
      Platform: Linux-6.12.13-200.fc41.x86_64-x86_64-with-glibc2.40
      Release: 6.12.13-200.fc41.x86_64
      Machine: x86_64
    pyinfra: v3.2
      click: v8.1.8
      distro: v1.9.0
      gevent: v24.11.1
      jinja2: v3.1.5
      packaging: v24.2
      paramiko: v3.5.1
      python-dateutil: v2.9.0.post0
      pywinrm: v0.5.0
      setuptools: v75.8.0
      typeguard: v4.4.1
      typing-extensions: v4.12.2
    Executable: <...>/.venv/bin/pyinfra
    Python: 3.13.2 (CPython, GCC 14.2.1 20250110 (Red Hat 14.2.1-7))
  • How was pyinfra installed (source/pip)?
    Installed locally to project using python poetry (installs to venv with pip).

  • Include pyinfra-debug.log (if one was created)

  File "<...>/.venv/lib/python3.13/site-packages/pyinfra_cli/main.py", line 222, in cli
    _main(*args, **kwargs)
    ~~~~~^^^^^^^^^^^^^^^^^
  File "<...>/.venv/lib/python3.13/site-packages/pyinfra_cli/main.py", line 341, in _main
    print_inventory(state)
    ~~~~~~~~~~~~~~~^^^^^^^
  File "<...>/.venv/lib/python3.13/site-packages/pyinfra_cli/prints.py", line 118, in print_inventory
    click.echo(jsonify(host.data, indent=4, default=json_encode), err=True)
               ~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "<...>/.venv/lib/python3.13/site-packages/pyinfra_cli/prints.py", line 52, in jsonify
    return json.dumps(data, *args, **kwargs)
           ~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/lib64/python3.13/json/__init__.py", line 238, in dumps
    **kw).encode(obj)
          ~~~~~~^^^^^
  File "/usr/lib64/python3.13/json/encoder.py", line 200, in encode
    chunks = self.iterencode(o, _one_shot=True)
  File "/usr/lib64/python3.13/json/encoder.py", line 261, in iterencode
    return _iterencode(o, 0)
  File "<...>/.venv/lib/python3.13/site-packages/pyinfra_cli/util.py", line 130, in json_encode
    raise TypeError("Cannot serialize: {0} ({1})".format(type(obj), obj))
TypeError: Cannot serialize: <class 'pyinfra.context.ContextObject'> (<pyinfra.api.inventory.Inventory object at 0x7f56f865c550>)
@david-d-white
Copy link
Author

david-d-white commented Feb 19, 2025

Commenting more information so that the bug post isn't huge:

I am of the opinion that including imports as inventory data is unexpected behavior. It also seems this way since the docs mention using from infra import inventory to access host data in group files.

I took a look and pyinfra_cli uses built in functions compile() and exec() to execute python files, uses the __all__ attribute to extract any variables that are defined, and ignores any attributes that start with "__" to ignore built in attributes. Since imports in python are bound locally, they will always be included in __all__.

I see two ways to fix this:

  1. The AbstractSyntaxTree module can be used in pyinfra_cli.util.exec_file() to walk the file and explicitly remove any imports from the data dictionary returned by the method. I did a hacky test by directly editing the pyinfra code in my venv, and this solved the problem and successfully removed imports from inventory data.

  2. A config option could be added to have pyinfra look for a specific dictionary such as data = {} instead of using everything in __all__ as inventory data.

I'm happy to work on a pull request for either of the above solutions if they seem acceptable, or for any other solutions that might be suggested.

@david-d-white
Copy link
Author

It should also be noted that a workaround is to alias any imports with a double underscore (e.g __alias) since pinfra doesn't include any attributes prefixed with a double underscore as inventory data. For example: import module as __alias or from package import module as __alias will both prevent module from being included as inventory data.

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

No branches or pull requests

1 participant