Skip to content

Fix: return error when update affects zero rows in execution repo#7283

Closed
muskan-creates352 wants to merge 1 commit into
flyteorg:masterfrom
muskan-creates352:fix-internal-runservice-7257
Closed

Fix: return error when update affects zero rows in execution repo#7283
muskan-creates352 wants to merge 1 commit into
flyteorg:masterfrom
muskan-creates352:fix-internal-runservice-7257

Conversation

@muskan-creates352

Copy link
Copy Markdown

Fixes #7257

This PR ensures that Update operations in ExecutionRepo return an error when no rows are affected.

Previously, updates silently succeeded even if no matching record existed, which could lead to hidden data issues.

This change adds a check on RowsAffected and returns a not found error when no rows are updated.

@codecov

codecov Bot commented Apr 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 58.53%. Comparing base (3fe1a5e) to head (2031b31).
⚠️ Report is 2 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #7283      +/-   ##
==========================================
+ Coverage   56.58%   58.53%   +1.95%     
==========================================
  Files         931      703     -228     
  Lines       64866    41189   -23677     
==========================================
- Hits        36707    24112   -12595     
+ Misses      25105    14944   -10161     
+ Partials     3054     2133     -921     
Flag Coverage Δ
unittests-datacatalog 53.51% <ø> (ø)
unittests-flyteadmin ?
unittests-flytecopilot 43.06% <ø> (+1.78%) ⬆️
unittests-flytectl 64.09% <ø> (ø)
unittests-flyteidl 75.71% <ø> (+2.49%) ⬆️
unittests-flyteplugins 60.19% <100.00%> (+0.85%) ⬆️
unittests-flytepropeller 53.71% <ø> (+0.33%) ⬆️
unittests-flytestdlib 62.61% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ 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.

@muskan-creates352

muskan-creates352 commented Apr 25, 2026

Copy link
Copy Markdown
Author

Hi ,

This PR ensures that update operations return an error when no rows are affected, preventing silent failures.

The fix has been applied consistently across ExecutionRepo, TaskExecutionRepo, and NodeExecutionRepo.

Please let me know if any further changes are needed. Thanks!

@muskan-creates352 muskan-creates352 force-pushed the fix-internal-runservice-7257 branch 4 times, most recently from c22101c to 8e655f3 Compare April 26, 2026 05:46
@muskan-creates352

Copy link
Copy Markdown
Author

Hi,

Could you please approve the workflows so that the checks can run?

Thanks!

@muskan-creates352 muskan-creates352 force-pushed the fix-internal-runservice-7257 branch from 8e655f3 to 67563e0 Compare April 26, 2026 06:27
Signed-off-by: Muskan Kumari <er.muskan09@gmail.com>
@muskan-creates352 muskan-creates352 force-pushed the fix-internal-runservice-7257 branch from 67563e0 to a002f88 Compare April 26, 2026 06:34
@Sovietaced

Copy link
Copy Markdown
Member

The issue you linked to references code in the v2 branch but you've fixed some unrelated code in the master branch.

@Sovietaced Sovietaced closed this Apr 30, 2026
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.

InternalRunService: silent data loss when RecordAction fails before UpdateActionStatus

2 participants