Skip to content

Conversation

@tzing
Copy link
Contributor

@tzing tzing commented Sep 16, 2025

Fixes #13718

Motivation

On using withParam/withItems on DAG/Steps, Argo Workflow mistakenly replace the \n chars as \\n and it may break some tasks.

Modifications

The operator used a JSON serialized string for building items. This casues some escape string got escaped again.
This PR use the the GetStrVal method to extract its original value.

Verification

Covered in the test cases added in fb957c6

Documentation

@tzing tzing force-pushed the fix/withparam-escape branch from 51a5ad0 to e50be2e Compare September 16, 2025 09:48
@tzing tzing force-pushed the fix/withparam-escape branch 2 times, most recently from 7788c6e to 1641d2b Compare December 6, 2025 03:54
@tzing tzing mentioned this pull request Dec 6, 2025
4 tasks
@tzing tzing force-pushed the fix/withparam-escape branch from 1641d2b to abfa425 Compare December 6, 2025 05:11
@tzing tzing force-pushed the fix/withparam-escape branch from abfa425 to fb957c6 Compare December 6, 2025 05:33
@tzing
Copy link
Contributor Author

tzing commented Dec 6, 2025

Note for the failed tests

On Dec 2025, I rebased the branch to reflect the changes from 2672ba3 and found the test "CI / E2E Tests (test-python-sdk)" failed.

This PR is focused on the logic in processItem, without changing any signature. I think the failed test is not related.

@MasonM MasonM self-assigned this Dec 8, 2025
@MasonM
Copy link
Member

MasonM commented Dec 9, 2025

/retest

1 similar comment
@eduardodbr
Copy link
Member

/retest

@MasonM
Copy link
Member

MasonM commented Dec 10, 2025

Thanks for the thorough tests! You're correct that the failures with CI / E2E Tests (test-python-sdk) were unrelated, and those were addressed in #14995. Now E2E Tests (test-examples, minimal, false, 0) is failing, and I'm pretty sure that's also unrelated. I believe the root cause is a race condition introduced in #15063, and I'm currently working on fixing those tests, hopefully by the end of the week.

@MasonM
Copy link
Member

MasonM commented Dec 15, 2025

/retest

Copy link
Member

@MasonM MasonM left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job @tzing! I appreciate your persistence

@MasonM MasonM merged commit 76eda6f into argoproj:main Dec 15, 2025
67 of 69 checks passed
@tzing tzing deleted the fix/withparam-escape branch December 16, 2025 02:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parameters values escaping problem

3 participants