flyte icon indicating copy to clipboard operation
flyte copied to clipboard

don't overwrite existing err when closing reader (#719)

Open pvditt opened this issue 1 month ago • 2 comments

Why are the changes needed?

Error on failed un-marshaling of an outputs file gets overwritten by deferred closing of the reader + swallow errors closing reader

What changes were proposed in this pull request?

don't overwrite err

How was this patch tested?

running in Union clusters for a while

Labels

Please add one or more of the following labels to categorize your PR:

  • added: For new features.
  • changed: For changes in existing functionality.
  • deprecated: For soon-to-be-removed features.
  • removed: For features being removed.
  • fixed: For any bug fixed.
  • security: In case of vulnerabilities

This is important to improve the readability of release notes.

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

pvditt avatar Nov 20 '25 22:11 pvditt

Bito Automatic Review Skipped - Draft PR

Bito didn't auto-review because this pull request is in draft status.
No action is needed if you didn't intend for the agent to review it. Otherwise, to manually trigger a review, type /review in a comment and save.
You can change draft PR review settings here, or contact your Bito workspace admin at [email protected].

flyte-bot avatar Nov 20 '25 22:11 flyte-bot

Codecov Report

:x: Patch coverage is 50.00000% with 1 line in your changes missing coverage. Please review. :white_check_mark: Project coverage is 59.70%. Comparing base (bca4499) to head (8b6176b).

Files with missing lines Patch % Lines
flytestdlib/storage/protobuf_store.go 50.00% 0 Missing and 1 partial :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6756      +/-   ##
==========================================
- Coverage   59.71%   59.70%   -0.02%     
==========================================
  Files         929      929              
  Lines       58011    58011              
==========================================
- Hits        34642    34636       -6     
- Misses      20214    20220       +6     
  Partials     3155     3155              
Flag Coverage Δ
unittests-datacatalog 60.30% <ø> (ø)
unittests-flyteadmin 57.83% <ø> (-0.04%) :arrow_down:
unittests-flytecopilot 43.16% <ø> (ø)
unittests-flytectl 65.31% <ø> (ø)
unittests-flyteidl 78.64% <ø> (ø)
unittests-flyteplugins 62.04% <ø> (ø)
unittests-flytepropeller 55.55% <ø> (ø)
unittests-flytestdlib 64.02% <50.00%> (ø)

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

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

codecov[bot] avatar Nov 20 '25 22:11 codecov[bot]