Skip to content

Commit

Permalink
fix: Lazy loading fails when evaluating nested vars (#98)
Browse files Browse the repository at this point in the history
* Fix lazy loading nested vars

* bump version v24.37.1 -> v24.37.2
  • Loading branch information
adammcdonagh committed Sep 10, 2024
1 parent 5d9dce1 commit 5c0c321
Show file tree
Hide file tree
Showing 5 changed files with 70 additions and 24 deletions.
44 changes: 22 additions & 22 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
"configurations": [
{
"name": "Python: Pytest All",
"type": "python",
"type": "debugpy",
"request": "launch",
"program": "-m",
"console": "integratedTerminal",
Expand All @@ -15,7 +15,7 @@
},
{
"name": "Python: Current File",
"type": "python",
"type": "debugpy",
"request": "launch",
"program": "${file}",
"preLaunchTask": "Build Test containers",
Expand All @@ -25,7 +25,7 @@
},
{
"name": "Python: Transfer - Basic",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -35,7 +35,7 @@
},
{
"name": "Python: Transfer - Basic - As job",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -45,7 +45,7 @@
},
{
"name": "Python: Transfer - SSH Vars",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -55,7 +55,7 @@
},
{
"name": "Python: Transfer - Basic - JSON Logging",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -68,7 +68,7 @@
},
{
"name": "Python: Batch x15",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -78,7 +78,7 @@
},
{
"name": "Python: Transfer - Multiple",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -88,7 +88,7 @@
},
{
"name": "Python: Transfer - File Conditions 1",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -98,7 +98,7 @@
},
{
"name": "Python: Transfer - File Watch only",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -108,7 +108,7 @@
},
{
"name": "Python: Transfer - Log Watch only",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -118,7 +118,7 @@
},
{
"name": "Python: Transfer - Log Watch tail only",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -128,7 +128,7 @@
},
{
"name": "Python: Batch - Basic",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -138,7 +138,7 @@
},
{
"name": "Python: Batch - Parallel",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -148,7 +148,7 @@
},
{
"name": "Python: Batch - Timeout",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -167,7 +167,7 @@
},
{
"name": "Python: Batch - Dependencies",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -186,7 +186,7 @@
},
{
"name": "Python: Batch - Dependencies - Fail - No retry",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -205,7 +205,7 @@
},
{
"name": "Python: Batch - Dependencies - Fail - Retry",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -224,7 +224,7 @@
},
{
"name": "Python: Batch - Continue on fail",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -234,7 +234,7 @@
},
{
"name": "Python: Execution - Basic - df - 2 hosts",
"type": "python",
"type": "debugpy",
"request": "launch",
"preLaunchTask": "Build Test containers",
"program": "src/opentaskpy/cli/task_run.py",
Expand All @@ -244,7 +244,7 @@
},
{
"name": "Python: Execution - Basic - df - local",
"type": "python",
"type": "debugpy",
"request": "launch",
"program": "src/opentaskpy/cli/task_run.py",
"console": "integratedTerminal",
Expand All @@ -253,7 +253,7 @@
},
{
"name": "Python: Bad variables file",
"type": "python",
"type": "debugpy",
"request": "launch",
"program": "src/opentaskpy/cli/task_run.py",
"console": "integratedTerminal",
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

# v24.37.2

- Fix issue where nested variables were not being resolved correctly when using lazy loading

# v24.37.1

- Allow loading of multiple variables files via `OTF_VARIABLES_FILE` by specifying them comma-separated
Expand Down
4 changes: 2 additions & 2 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"

[project]
name = "opentaskpy"
version = "v24.37.1"
version = "v24.37.2"
authors = [{ name = "Adam McDonagh", email = "adam@elitemonkey.net" }]
license = { text = "GPLv3" }
classifiers = [
Expand Down Expand Up @@ -71,7 +71,7 @@ otf-batch-validator = "opentaskpy.cli.batch_validator:main"
profile = 'black'

[tool.bumpver]
current_version = "v24.37.1"
current_version = "v24.37.2"
version_pattern = "vYY.WW.PATCH[-TAG]"
commit_message = "bump version {old_version} -> {new_version}"
commit = true
Expand Down
15 changes: 15 additions & 0 deletions src/opentaskpy/config/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,16 @@ def _enrich_variables(self, task_definition_file: str) -> dict:

unresolved_variable = self.global_variables[undeclared_variable]

# Check the type of the variable, if it's not a string, then dump it into a JSON string
converted_variable = False
if not isinstance(unresolved_variable, str):
self.logger.debug(
"Converted unresolved variable into a RAW string"
)
unresolved_variable = json.dumps(unresolved_variable)
converted_variable = True

self.logger.info(f"Resolving variable {undeclared_variable}")
evaluated_variable = self._resolve_templated_variables_from_string(
unresolved_variable
)
Expand All @@ -293,6 +303,11 @@ def _enrich_variables(self, task_definition_file: str) -> dict:
f"Resolved variable {undeclared_variable}",
)

# If the variable was not a string, then convert it back to the original type
if converted_variable:
self.logger.debug("Converting variable back into a JSON object")
evaluated_variable = json.loads(evaluated_variable)

# Now update the global variables with the resolved value
self.global_variables[undeclared_variable] = evaluated_variable

Expand Down
27 changes: 27 additions & 0 deletions tests/test_config_loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,33 @@ def test_load_task_definition(write_dummy_variables_file, tmpdir):
assert e.value.args[0] == "Couldn't find task with name: task"


def test_lazy_loading_nested_variables(tmpdir):
os.environ["OTF_LAZY_LOAD_VARIABLES"] = "1"
# Create a variables file with some test variables in it
fs.create_files(
[
{
f"{tmpdir}/variables.json.j2": {
"content": '{"NEST": { "NEST1": "{{ now().strftime(\'%Y\') }}" } } '
}
},
]
)

# Create a task definition file (this isn't valid, but it proves if the evaluation of variables works)
fs.create_files(
[{f"{tmpdir}/task.json": {"content": '{"test": "{{ NEST.NEST1 }}"}'}}]
)

evaluated_year = datetime.now().strftime("%Y")
expected_task_definition = {"test": f"{evaluated_year}"}

# Test that the task definition is loaded correctly
config_loader = ConfigLoader(tmpdir)
del os.environ["OTF_LAZY_LOAD_VARIABLES"]
assert config_loader.load_task_definition("task") == expected_task_definition


def test_load_task_definition_lazy_load(tmpdir):
# Set the OTF_LAZY_LOAD_VARIABLES environment variable
os.environ["OTF_LAZY_LOAD_VARIABLES"] = "1"
Expand Down

0 comments on commit 5c0c321

Please sign in to comment.