Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Truncate execution error message in list execution calls #523

Merged
merged 2 commits into from
Feb 16, 2023

Conversation

katrogan
Copy link
Contributor

TL;DR

For repeated execution failures with long error message stack traces, the call to list executions can exceed the default gRPC message size.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

flyteorg/flyte#3337

Follow-up issue

NA

Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
Signed-off-by: Katrina Rogan <katroganGH@gmail.com>
@codecov
Copy link

codecov bot commented Feb 15, 2023

Codecov Report

Merging #523 (b8e1238) into master (2e156a3) will increase coverage by 0.07%.
The diff coverage is 94.16%.

@@            Coverage Diff             @@
##           master     #523      +/-   ##
==========================================
+ Coverage   60.09%   60.17%   +0.07%     
==========================================
  Files         168      169       +1     
  Lines       15065    15095      +30     
==========================================
+ Hits         9054     9084      +30     
  Misses       5211     5211              
  Partials      800      800              
Flag Coverage Δ
unittests 60.17% <94.16%> (+0.07%) ⬆️

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

Impacted Files Coverage Δ
pkg/manager/impl/util/resources.go 90.66% <90.66%> (ø)
pkg/manager/impl/execution_manager.go 70.42% <90.90%> (-0.96%) ⬇️
pkg/manager/impl/node_execution_manager.go 69.93% <100.00%> (ø)
pkg/manager/impl/task_execution_manager.go 69.28% <100.00%> (ø)
pkg/manager/impl/task_manager.go 64.53% <100.00%> (+0.53%) ⬆️
pkg/manager/impl/validation/task_validator.go 86.60% <100.00%> (ø)
pkg/repositories/transformers/execution.go 79.86% <100.00%> (+0.50%) ⬆️
pkg/repositories/transformers/node_execution.go 70.43% <100.00%> (+0.77%) ⬆️
pkg/repositories/transformers/task_execution.go 72.37% <100.00%> (+0.54%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

Copy link
Contributor

@wild-endeavor wild-endeavor left a comment

Choose a reason for hiding this comment

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

but is 100 too small? this is in bytes? trimmedErrOutputResult.Message[0:trimmedErrMessageLen] will just return the first 100 bytes?

@katrogan
Copy link
Contributor Author

Yeah, the primary use-case case for this endpoint is predominantly listing execution phases in the UI - output results are discarded. To render the full error message/output we usually land on the individual execution page which should call GetExecution to get details in full

@katrogan katrogan merged commit 8596a56 into master Feb 16, 2023
@katrogan katrogan deleted the trim-exec-err-msg branch February 16, 2023 00:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants