fix(api): include name and arguments in operation hash - #1942
Open
wowi42 wants to merge 3 commits into
Open
Conversation
Operations at the same position (eg calls in a plain loop with per-host data) previously shared one hash, merging their names and host counts in the detected changes output and breaking _run_once/_parallel semantics. Hash the operation names and arguments alongside the call position so distinct operations get distinct hashes, while identical operations across hosts still share one. Fixes pyinfra-dev#1370
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1370, implementing the approach from this analysis.
Problem
solve_operation_consistencybuilt the operation hash purely from position (stack lines in CLI mode, per-host index in API mode, plushost.loop_position). Names and arguments were never hashed, and the duplicate-resolution loop only checked the current host'sop_hash_order. As a result, different operations at the same position (eg calls in a plainforloop with per-host data) collapsed into a single hash:ensure_shared_op_metaunioned their names into one set, so the "Detected changes" output showed a nondeterministic name and merged host counts_parallel,_run_onceor_serial, the deploy died withOperationValueError: Cannot have different values for ..._run_oncesemantics broke, running the merged op on the first host onlyFix
Hash the operation names and arguments (normalized via
_get_arg_value, so functions andStringIOstay deterministic) alongside the position. Different operations at the same position now get distinct hashes, while identical operations across hosts still share one hash, keeping cross-host dedup and_run_once/_serial/_parallelgrouping intact. The same-host duplicate suffix behavior (hash-0,hash-0-1, ...) is unchanged.Tests
test_op_per_host_data_no_hash_collision- per-host named ops get distinct hashes; identical ops still share onetest_op_per_host_args_no_name_no_hash_collision- same op/position with per-host args but no explicitname=test_op_per_host_data_different_execution_kwargs- no falseOperationValueErrorfor distinct ops with different_paralleltest_cli_op_loop_per_host_data_no_hash_collision- exact issue repro in CLI mode (plain loop, same line)test_cli_op_loop_identical_ops_dedupe- identical same-line loop calls still dedupe via hash suffixesAll collision tests fail on the old code; the dedup test passes on both.