pyinfra-dev / pyinfra

pyinfra turns Python code into shell commands and runs them on your servers. Execute ad-hoc commands and write declarative operations. Target SSH servers, local machine and Docker containers. Fast and scales from one server to thousands.
https://pyinfra.com
MIT License
3.91k stars 383 forks source link

Nested local.include doesn't work as expected #453

Closed taranlu-houzz closed 3 years ago

taranlu-houzz commented 4 years ago

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:

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))
Fizzadar commented 4 years ago

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.

Fizzadar commented 3 years ago

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!

Fizzadar commented 3 years ago

Now released in v1.3.5!

taranlu-houzz commented 3 years ago

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?

Fizzadar commented 3 years ago

👍 I've added this @ https://github.com/Fizzadar/pyinfra/commit/ffa70fe5f4e408a6c2d123876467675a58db52a6!

taranlu-houzz commented 3 years ago

@Fizzadar Not sure if I am doing something wrong (or if it is not meant to be used this way), but it seems that using the local.include with a relative path multiple times does not work. If I have a deploy where I would like to group several other deploys (basically sub-tasks), I get an error where it cannot seem to resolve the path correctly. This is my deploy:

#!/usr/bin/env python
# -*- coding: utf-8 -*-

"""Pyinfra deploy to install and setup perforce client."""

####################################################################################################
# Imports
####################################################################################################

# ==Site-Packages==
from pyinfra import local

####################################################################################################
# Main
####################################################################################################

local.include("../tasks/perforce/install_p4_cli.py")
local.include("../tasks/perforce/install_p4_gui.py")

This is the dir structure:

pyinfra/
├── data
├── deploys
│   ├── roles
│   │   ├── ...
│   │   ├── perforce_client.py
│   │   └── ...
│   └── tasks
│       ├── ...
│       ├── perforce
│       │   ├── install_p4_cli.py
│       │   └── install_p4_gui.py
│       └── ...
├── facts
├── group_data
├── inventories
└── operations

And when I run pyinfra, I get this output:

↴2 ----/pyinfra on  pyinfra [✘!?⇡]
❯ pyinfra inventories/---- deploys/roles/perforce_client.py
--> Loading config...
--> Loading inventory...

--> Connecting to hosts...
    [----] Connected

--> Preparing operations...
    Loading: deploys/roles/perforce_client.py
--> An unexpected exception occurred in: deploys/roles/perforce_client.py:

  File "----/lib/python3.9/site-packages/pyinfra_cli/util.py", line 79, in exec_file
    exec(PYTHON_CODES[filename], data)
  File "deploys/roles/perforce_client.py", line 23, in <module>
    local.include("../tasks/perforce/install_p4_gui.py")
  File "----/lib/python3.9/site-packages/pyinfra/local.py", line 57, in include
    raise PyinfraError(
Traceback (most recent call last):
  File "----/lib/python3.9/site-packages/pyinfra/local.py", line 41, in include
    config_data = extract_file_config(filename)
  File "----/lib/python3.9/site-packages/pyinfra_cli/config.py", line 13, in extract_file_config
    with open(filename, 'r') as f:
FileNotFoundError: [Errno 2] No such file or directory: 'deploys/roles/../tasks/perforce/../tasks/perforce/install_p4_gui.py'

During handling of the above exception, another exception occurred:

pyinfra.api.exceptions.PyinfraError: Could not include local file: deploys/roles/../tasks/perforce/../tasks/perforce/install_p4_gui.py:
[Errno 2] No such file or directory: 'deploys/roles/../tasks/perforce/../tasks/perforce/install_p4_gui.py'
Fizzadar commented 3 years ago

@taranlu-houzz thank you for following up on this, there's a bug in the handling of exec file which I've just fixed in https://github.com/Fizzadar/pyinfra/commit/7e81e2475821498a0df97879c64c4fd537897d37 and released in v1.4.13.

taranlu-houzz commented 3 years ago

@Fizzadar Cool, I will try that out once v1.4.13 is released. Thanks!