Log and emit a warning when merged workflow outputs have name collisions

[woutdenolf]

PR summary

Part of ewoks-kit/ewokscore#510. Need upstream release with https://gitlab.esrf.fr/workflow/pypushflow/-/merge_requests/118.

AI Disclosure

• No AI used
• AI tool Claude used for investigation handling merging in pypushflow or in ewoksppf

[olofsvensson]

Hi @woutdenolf ,

I had a look at the changes and said to myself that this is one of the easiest code reviews that I have ever done. However... I rand the changes through claude who didn't think that it was that easy :

---

The ewoksppf branch isn't ready to merge. With the matching pypushflow branch installed, 6 tests that pass today fail under CI's -W error setting, and the new warning also fires for normal workflows and for an internal variable users can't rename.

I used pypushflow's origin/warn_output_name_collision (a0223d1) with ewokscore 5.0.0 on Python 3.13, in a scratch venv. Your pypushflow checkout is unchanged; I removed the temporary worktree afterwards.

Setup	Result
Released pypushflow 2.1.0rc1, -W error	3 failed: the 2 test_on_error_meta tests ("DID NOT RAISE TaskExecutionError") and test_all_outputs_merged, because that release has no warning
pypushflow branch, no -W error	2 failed: the same 2 test_on_error_meta tests
pypushflow branch, -W error (as CI runs it)	8 failed: the 2 test_on_error_meta tests plus 6 new ones

Main problems

1. Plain execute_graph() calls now warn. Without outputs=, ewoksppf returns the merged outputs of all end tasks. Any graph with more than one end task producing the same names (e.g. result) now warns, even though the user never asked for merged outputs. This is what breaks test_examples[triangle1] (both variants), test_workflow16, test_workflow18[True] and test_outputs_like_sequential[merge-outputs3]. Either the collision warning should only apply to explicit output selections, or ewoksppf needs to handle this default case.

2. The warning names _ppfdict, an internal ewoksppf variable. Asking for all outputs of a task (OutputSelection() with no name) copies its whole result, including _ppfdict. So test_workflow16 and one triangle1 case warn about '_ppfdict' ... overwritten. Users can't sensibly rename that with new_name. ewoksppf should keep _ppfdict out of the outputs it returns, or pypushflow should not warn about it.

3. A warning can abort the workflow. store_result runs inside the actor's execution path. When warnings are errors (as in CI, or any application that sets that), the UserWarning becomes an exception and the workflow stops ("Interrupt workflow execution: Merged workflow results …"). Collecting results should never kill a run. pypushflow could catch exceptions around the warning, or only log in store_result and emit the warning once after the run finishes, in the calling thread.

4. test_all_outputs_merged fails for an unrelated reason. On Python 3.12+, pytest.warns also picks up a DeprecationWarning ("process is multi-threaded, use of fork() may lead to deadlocks") and passes it back out. Under -W error that fails the test even though the expected warning is raised. That DeprecationWarning needs filtering in this test.

---

[woutdenolf]

• Default execute_graph() warning: intended. The core engine does the same since Log and emit a warning when merged workflow outputs have name collisions ewokscore#512, so I updated the tests.
• _ppfdict: not internal, it's the output of ppf tasks and users get it back. The overwrite is real, so the tests now expect the warning.
• Warning aborting the workflow: fixed in pypushflow. Collisions are logged during the run and the warning is emitted after it, in the calling thread.
• Fork DeprecationWarning: filtered in the tests that use pytest.warns.
• test_on_error_meta: unrelated, passes with ewokscore main.

Pushed to both branches.

[woutdenolf]

Need upstream release with https://gitlab.esrf.fr/workflow/pypushflow/-/merge_requests/118

[olofsvensson]

Looks good to me.

[olofsvensson]

Ok for me.