Pyinfra: Nested local.include doesn't work as expected

Created on 16 Oct 2020  ยท  5Comments  ยท  Source: Fizzadar/pyinfra

Description
It seems that local.include always evaluates the path relative to the top level deploy, or rather, the one specified on the command line. This causes issues with nested deploys that include other deploys, which in turn contain others if some of these deploys are in a different location on the directory tree.

For example, with this structure:

.
โ”œโ”€โ”€ data
โ”œโ”€โ”€ deploys
โ”‚ย ย  โ”œโ”€โ”€ facts
โ”‚ย ย  โ”œโ”€โ”€ roles
โ”‚ย ย  โ”‚ย ย  โ””โ”€โ”€ group_data -> ../../group_data
โ”‚ย ย  โ””โ”€โ”€ tasks
โ”‚ย ย      โ””โ”€โ”€ group_data -> ../../group_data
โ”œโ”€โ”€ group_data
โ””โ”€โ”€ inventories

The tasks dir is used to store building-block deploys for reuse which may need to be called in a very specific order, while the roles dir contains deploys made up of groupings of tasks (and other additional operations) that are "guaranteed" to work together.

This issue crops up in the following scenario:

  • Deploys A.py and B.py are in tasks.
  • A.py uses local.include to include B.py.
  • Deploy C.py is in roles.
  • C.py uses local.include to include A.py.

This causes an error when trying to run pyinfra <inventory> deploys/roles/C.py, because the local.include in A.py attempts to use a relative path based on the location of C.py, which is invalid.

Expected behavior
I feel like it makes more sense for local.include to evaluate the path relative to the location of the deploy file that it is in so that nesting would "work as expected."

Meta

โฏ pyinfra --support
--> Support information:

    If you are having issues with pyinfra or wish to make feature requests, please
    check out the GitHub issues at https://github.com/Fizzadar/pyinfra/issues .
    When adding an issue, be sure to include the following:

    System: Darwin
      Platform: Darwin-18.7.0-x86_64-i386-64bit
      Release: 18.7.0
      Machine: x86_64
    pyinfra: v1.1.2
    Executable: /Users/----/.local/bin/pyinfra
    Python: 3.7.8 (CPython, Clang 10.0.1 (clang-1001.0.46.4))
  • Installed via pipx
Bug

All 5 comments

Hi @taranlu-houzz, thanks for this issue!

I feel like it makes more sense for local.include to evaluate the path relative to the location of the deploy file that it is in so that nesting would "work as expected."

Agreed, this definitely feels like the correct behaviour - have never considered nested includes properly. I'm interested to investigate if the current (relative to deploy directory) behaviour is expected anywhere but pro this change.

I have now got a working implementation of this that maintains backwards compatibility (https://github.com/Fizzadar/pyinfra/pull/522). This can be used by explicitly specifying a relative path, ie local.include('./another_file.py').

Aiming to release this in v1.3.5 this weekend!

Now released in v1.3.5!

Seems to be working, but the logging looks odd because it keeps appending the dirname of the relative path multiple times in some cases. Here is an example (still works correctly though):

--> Starting operation: /Users/----/pyinfra/deploys/tasks/supervisor/../../tasks/supervisor/../../tasks/pipeline/../../tasks/pipeline/../../tasks/pipeline/../../tasks/pipeline/../../tasks/pipeline/../../tasks/supervisor/start_service.py | Start supervisor service as root.
    [----] Success

Maybe it would make sense to only show the abspath for the included file?

Was this page helpful?
0 / 5 - 0 ratings