Support bound inputs for array node tasks #3185

pvditt · 2025-03-11T06:35:26Z

Tracking issue

fixes: https://linear.app/unionai/issue/BB-2941/utilize-bound-inputs-for-partials-for-map-tasks

Why are the changes needed?

What changes were proposed in this pull request?

add support for setting bound_inputs for ArrayNodeMapTask

How was this patch tested?

https://dogfood.cloud-staging.union.ai/console/projects/flytesnacks/domains/development/executions/a29rf95w9fx4hvqjpnt8/nodeId/n0/nodes

@workflow
def wf():
    strings = ['hello', 'world']
    strings1 = ['hello', 'world']
    return union.map(my_task, bound_inputs={"s1": "test"})(s=strings, s1=strings1)

Setup process

Screenshots

Check all the applicable boxes

I updated the documentation accordingly.
All new and existing tests passed.
All commits are signed-off.

Related PRs

Docs link

Summary by Bito

This PR adds support for bound inputs in ArrayNodeMapTask with input consistency validation and improves input validation. The implementation includes updates to the flytekit core and translator module, implementing checks for key mismatches between 'bound_inputs' and 'bound_inputs_values'. Unit tests were updated to remove try-except blocks for cleaner error detection, enhancing robustness and flexibility in map task handling.

Unit tests added: True

Estimated effort to review (1-5, lower is better): 2

Signed-off-by: Paul Dittamo <[email protected]>

flyte-bot · 2025-03-11T06:35:41Z

Code Review Agent Run Status

Limitations and other issues: ❌ Failure - The AI Code Review Agent skipped reviewing this change because it is configured to exclude certain pull requests based on the source/target branch or the pull request status. You can change the settings here, or contact the agent instance creator at [email protected].

Signed-off-by: Paul Dittamo <[email protected]>

This reverts commit 89c5895. Signed-off-by: Paul Dittamo <[email protected]>

codecov · 2025-03-11T07:19:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 73.15%. Comparing base (0214448) to head (31a7ee2).
Report is 3 commits behind head on master.

❗ There is a different number of reports uploaded between BASE (0214448) and HEAD (31a7ee2). Click for more details.

HEAD has 23 uploads less than BASE

Flag BASE (0214448) HEAD (31a7ee2)

26 3

Additional details and impacted files

@@             Coverage Diff             @@
##           master    #3185       +/-   ##
===========================================
- Coverage   89.32%   73.15%   -16.17%     
===========================================
  Files         134      213       +79     
  Lines        6136    22301    +16165     
  Branches        0     2914     +2914     
===========================================
+ Hits         5481    16315    +10834     
- Misses        655     5169     +4514     
- Partials        0      817      +817

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

flyte-bot · 2025-03-11T07:53:00Z

Code Review Agent Run #f8b189

Actionable Suggestions - 1

flytekit/core/array_node_map_task.py - 1
- Potential comparison issue with set and dict · Line 86-86

Review Details

Files reviewed - 3 · Commit Range: 3220eae..7af739a
- flytekit/core/array_node_map_task.py
- flytekit/tools/translator.py
- tests/flytekit/unit/core/test_array_node_map_task.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

AI Code Review powered by

flyte-bot · 2025-03-11T07:58:13Z

Changelist by Bito

This pull request implements the following key changes.

Key Change	Files Impacted
New Feature - Introduce Bound Inputs for ArrayNodeMapTask	- `array_node_map_task.py` - Added a new parameter 'bound_inputs_values' to support bound inputs, including consistency checks with 'bound_inputs' and updates to init and call methods. - `translator.py` - Updated serialization function to include the 'bound_inputs' parameter for ArrayNodeMapTask, ensuring proper task definition propagation.
Testing - Add Tests for Bound Inputs Functionality	- `test_array_node_map_task.py` - Expanded test suite to validate collision rules, input overrides, and correct serialization for new bound inputs support in various workflow scenarios.

flyte-bot · 2025-03-11T07:58:15Z

flytekit/core/array_node_map_task.py

+        # Note, bound_inputs are passed in during run time when executing the task
+        # so both values shouldn't be set at the same time
+        if bound_inputs and bound_inputs_values:
+            if bound_inputs != set(bound_inputs_values):


Potential comparison issue with set and dict

The condition bound_inputs != set(bound_inputs_values) might not work as expected when comparing a set with dictionary keys. Consider using bound_inputs != set(bound_inputs_values.keys()) to ensure proper comparison between the set and dictionary keys.

Code suggestion

Check the AI-generated fix before applying

Suggested change

if bound_inputs != set(bound_inputs_values):

if bound_inputs != set(bound_inputs_values.keys()):

Code Review Run #f8b189

Should Bito avoid suggestions like this for future reviews? (Manage Rules)

Yes, avoid them

thomasjpfan · 2025-03-11T14:23:36Z

flytekit/core/array_node_map_task.py

+        # so both values shouldn't be set at the same time
+        if bound_inputs and bound_inputs_values:
+            if bound_inputs != set(bound_inputs_values):
+                raise ValueError("bound_inputs and bound_inputs_values should have the same keys if both set")


Can we include a test that triggers this error?

Signed-off-by: Paul Dittamo <[email protected]>

flyte-bot · 2025-03-11T18:22:48Z

Code Review Agent Run #d4467d

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: 7af739a..9252ce8
- tests/flytekit/unit/core/test_array_node_map_task.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by

thomasjpfan · 2025-03-11T21:31:45Z

tests/flytekit/unit/core/test_array_node_map_task.py

+    with pytest.raises(ValueError, match="bound_inputs and bound_inputs_values should have the same keys if both set"):
+        ArrayNodeMapTask(task1, bound_inputs_values={"c": param_c}, bound_inputs={"b"})(a=param_a, b=param_b)
+
+    try:


Can this also use pytest.raises?

Also, how does the error get triggered from union.map?

@thomasjpfan Should we use pytest.raises here since it's not raising an error?

We shouldn't really ever hit this error. Have this PR that exposes bound_inputs param for union.map

Ah it's the other way around. In that case, I'm okay with just writing and let pytest see the original error.

# no error raised ArrayNodeMapTask(task1, bound_inputs_values={"c": param_c}, bound_inputs={"c"})(a=param_a, b=param_b)

thomasjpfan · 2025-03-12T15:23:36Z

tests/flytekit/unit/core/test_array_node_map_task.py

+    with pytest.raises(ValueError, match="bound_inputs and bound_inputs_values should have the same keys if both set"):
+        ArrayNodeMapTask(task1, bound_inputs_values={"c": param_c}, bound_inputs={"b"})(a=param_a, b=param_b)
+
+    try:


Ah it's the other way around. In that case, I'm okay with just writing and let pytest see the original error.

# no error raised ArrayNodeMapTask(task1, bound_inputs_values={"c": param_c}, bound_inputs={"c"})(a=param_a, b=param_b)

Signed-off-by: Paul Dittamo <[email protected]>

flyte-bot · 2025-03-12T18:22:51Z

Code Review Agent Run #22f506

Actionable Suggestions - 0

Review Details

Files reviewed - 1 · Commit Range: 9252ce8..31a7ee2
- tests/flytekit/unit/core/test_array_node_map_task.py
Files skipped - 0
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
- MyPy (Static Code Analysis) - ✔︎ Successful
- Astral Ruff (Static Code Analysis) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

/review - Manually triggers a full AI review.

Refer to the documentation for additional commands.

Configuration

This repository uses code_review_bito You can customize the agent settings here or contact your Bito workspace admin at [email protected].

Documentation & Help

AI Code Review powered by

pvditt added 2 commits March 10, 2025 23:30

support bound inputs in array node map tasks

3220eae

Signed-off-by: Paul Dittamo <[email protected]>

clean up

d22ad13

Signed-off-by: Paul Dittamo <[email protected]>

clean up

dc595a1

Signed-off-by: Paul Dittamo <[email protected]>

pvditt marked this pull request as ready for review March 11, 2025 06:49

pvditt requested review from wild-endeavor, kumare3, eapolinario, pingsutw, cosmicBboy, samhita-alla, thomasjpfan and Future-Outlier as code owners March 11, 2025 06:49

pvditt added 2 commits March 11, 2025 00:06

temp set error until changes are upstreamed

89c5895

Signed-off-by: Paul Dittamo <[email protected]>

Revert "temp set error until changes are upstreamed"

7af739a

This reverts commit 89c5895. Signed-off-by: Paul Dittamo <[email protected]>

flyte-bot reviewed Mar 11, 2025

View reviewed changes

pvditt mentioned this pull request Mar 11, 2025

upstream bound inputs support flyteorg/flyte#6322

Merged

3 tasks

thomasjpfan reviewed Mar 11, 2025

View reviewed changes

test

9252ce8

Signed-off-by: Paul Dittamo <[email protected]>

pvditt requested a review from thomasjpfan March 11, 2025 17:30

thomasjpfan reviewed Mar 11, 2025

View reviewed changes

pvditt requested a review from thomasjpfan March 11, 2025 22:08

thomasjpfan previously approved these changes Mar 12, 2025

View reviewed changes

test

31a7ee2

Signed-off-by: Paul Dittamo <[email protected]>

pvditt dismissed thomasjpfan’s stale review via 31a7ee2 March 12, 2025 17:18

pvditt requested a review from thomasjpfan March 12, 2025 17:33

thomasjpfan approved these changes Mar 12, 2025

View reviewed changes

pvditt enabled auto-merge (squash) March 12, 2025 17:35

pvditt merged commit af199b5 into master Mar 12, 2025
112 of 114 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support bound inputs for array node tasks #3185

Support bound inputs for array node tasks #3185

pvditt commented Mar 11, 2025 •

edited by flyte-bot

Loading

flyte-bot commented Mar 11, 2025

codecov bot commented Mar 11, 2025 •

edited

Loading

flyte-bot commented Mar 11, 2025 •

edited

Loading

Code Review Agent Run #f8b189

flyte-bot commented Mar 11, 2025 •

edited

Loading

Changelist by Bito

flyte-bot Mar 11, 2025

thomasjpfan Mar 11, 2025

pvditt Mar 11, 2025

flyte-bot commented Mar 11, 2025 •

edited

Loading

Code Review Agent Run #d4467d

Bito Usage Guide

thomasjpfan Mar 11, 2025

pvditt Mar 11, 2025

thomasjpfan Mar 12, 2025 •

edited

Loading

thomasjpfan Mar 12, 2025 •

edited

Loading

flyte-bot commented Mar 12, 2025 •

edited

Loading

Code Review Agent Run #22f506

Bito Usage Guide

	if bound_inputs != set(bound_inputs_values):
	if bound_inputs != set(bound_inputs_values.keys()):

Support bound inputs for array node tasks #3185

Support bound inputs for array node tasks #3185

Conversation

pvditt commented Mar 11, 2025 • edited by flyte-bot Loading

Tracking issue

Why are the changes needed?

What changes were proposed in this pull request?

How was this patch tested?

Setup process

Screenshots

Check all the applicable boxes

Related PRs

Docs link

Summary by Bito

flyte-bot commented Mar 11, 2025

Code Review Agent Run Status

codecov bot commented Mar 11, 2025 • edited Loading

Codecov Report

flyte-bot commented Mar 11, 2025 • edited Loading

Code Review Agent Run #f8b189

flyte-bot commented Mar 11, 2025 • edited Loading

Changelist by Bito

flyte-bot Mar 11, 2025

Choose a reason for hiding this comment

thomasjpfan Mar 11, 2025

Choose a reason for hiding this comment

pvditt Mar 11, 2025

Choose a reason for hiding this comment

flyte-bot commented Mar 11, 2025 • edited Loading

Code Review Agent Run #d4467d

Bito Usage Guide

thomasjpfan Mar 11, 2025

Choose a reason for hiding this comment

pvditt Mar 11, 2025

Choose a reason for hiding this comment

thomasjpfan Mar 12, 2025 • edited Loading

Choose a reason for hiding this comment

thomasjpfan Mar 12, 2025 • edited Loading

Choose a reason for hiding this comment

flyte-bot commented Mar 12, 2025 • edited Loading

Code Review Agent Run #22f506

Bito Usage Guide

pvditt commented Mar 11, 2025 •

edited by flyte-bot

Loading

codecov bot commented Mar 11, 2025 •

edited

Loading

flyte-bot commented Mar 11, 2025 •

edited

Loading

flyte-bot commented Mar 11, 2025 •

edited

Loading

flyte-bot commented Mar 11, 2025 •

edited

Loading

thomasjpfan Mar 12, 2025 •

edited

Loading

thomasjpfan Mar 12, 2025 •

edited

Loading

flyte-bot commented Mar 12, 2025 •

edited

Loading