flyteadmin icon indicating copy to clipboard operation
flyteadmin copied to clipboard

Make list execution transformer configurable

Open pradithya opened this issue 2 years ago • 3 comments

TL;DR

Add configuration in flyte admin to control the behavior of the list execution transformer with regard to error message trimming.

Type

  • [X] Bug Fix
  • [ ] Feature
  • [ ] Plugin

Are all requirements met?

  • [X] Code completed

Complete description

A hardcoded value of trimmedErrMessageLen is used to limit the error message shown in the workflow execution list. The current value is too small that the error message in the list execution page is not informative. Users must go to the corresponding execution details to get the entire error message even for a short error message. This PR add configuration in flyte admin to control whether the error message truncation is enabled and the maximum message length when truncation is enabled. The default is to truncate error messages for list execution API with a maximum error message length of 10240 so that most error messages can be shown properly.

https://user-images.githubusercontent.com/4023015/236148171-664ef81e-2525-4793-b0b1-8c18d0f7dab3.mov

Follow-up issue

NA

pradithya avatar May 04 '23 08:05 pradithya

Codecov Report

Merging #555 (59f783e) into master (1026231) will increase coverage by 1.65%. The diff coverage is 74.54%.

:exclamation: Current head 59f783e differs from pull request most recent head 3adb19b. Consider uploading reports for the commit 3adb19b to get more accurate results

@@            Coverage Diff             @@
##           master     #555      +/-   ##
==========================================
+ Coverage   58.85%   60.51%   +1.65%     
==========================================
  Files         170      172       +2     
  Lines       16068    13309    -2759     
==========================================
- Hits         9457     8054    -1403     
+ Misses       5777     4409    -1368     
- Partials      834      846      +12     
Flag Coverage Δ
unittests ?

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

Impacted Files Coverage Δ
pkg/runtime/application_config_provider.go 33.33% <ø> (ø)
pkg/common/artifacttype_enumer.go 40.00% <40.00%> (ø)
dataproxy/service.go 57.71% <63.09%> (+4.84%) :arrow_up:
pkg/common/flyte_url.go 96.00% <96.00%> (ø)
pkg/manager/impl/execution_manager.go 72.66% <100.00%> (+2.66%) :arrow_up:
pkg/manager/impl/node_execution_manager.go 72.92% <100.00%> (+2.92%) :arrow_up:
pkg/manager/impl/task_execution_manager.go 72.39% <100.00%> (+3.10%) :arrow_up:
pkg/repositories/transformers/execution.go 82.68% <100.00%> (+2.75%) :arrow_up:
pkg/repositories/transformers/node_execution.go 75.20% <100.00%> (+2.65%) :arrow_up:
pkg/repositories/transformers/task_execution.go 76.03% <100.00%> (+2.24%) :arrow_up:

... and 149 files with indirect coverage changes

codecov[bot] avatar May 04 '23 08:05 codecov[bot]

on second thought @pradithya - can you make this configurable? your new value as the default is fine, but it would be nice to be configurable.

Make sense, let me have a look.

pradithya avatar May 06 '23 06:05 pradithya

@wild-endeavor I updated the PR to make it configurable. But it seems the CI is failing due to Github action rate limit. Can you help retry it?

pradithya avatar May 06 '23 08:05 pradithya