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

Handle batched TaskExecutionEvent reasons #615

Merged
merged 5 commits into from
Sep 27, 2023
Merged

Handle batched TaskExecutionEvent reasons #615

merged 5 commits into from
Sep 27, 2023

Conversation

andrewwdye
Copy link
Contributor

TL;DR

Handle newly added TaskExecutionEvent.reasons field as part of flyteorg/flyteidl#443.

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

This change updates admin to process the recently added TaskExecutionEvent.reasons field, included to support sending batched reason events. This reduces load on both propeller and admin. Admin favors the reasons field but falls back to the reason field and prior behavior if empty.

Tracking Issue

flyteorg/flyte#3825

Follow-up issue

N/A

Signed-off-by: Andrew Dye <andrewwdye@gmail.com>
…k8s-events

Signed-off-by: Andrew Dye <andrewwdye@gmail.com>
Signed-off-by: Andrew Dye <andrewwdye@gmail.com>
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (dc8cc9d) 59.01% compared to head (6d58de7) 60.65%.
Report is 1 commits behind head on master.

❗ Current head 6d58de7 differs from pull request most recent head c049ced. Consider uploading reports for the commit c049ced to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #615      +/-   ##
==========================================
+ Coverage   59.01%   60.65%   +1.63%     
==========================================
  Files         171      171              
  Lines       16468    13469    -2999     
==========================================
- Hits         9719     8169    -1550     
+ Misses       5899     4449    -1450     
- Partials      850      851       +1     
Flag Coverage Δ
unittests ?

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

Files Coverage Δ
pkg/repositories/transformers/task_execution.go 77.81% <100.00%> (+3.88%) ⬆️
pkg/manager/impl/task_execution_manager.go 73.05% <0.00%> (+3.13%) ⬆️
pkg/repositories/transformers/workflow.go 74.60% <83.33%> (+7.93%) ⬆️

... and 155 files with indirect coverage changes

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

Signed-off-by: Andrew Dye <andrewwdye@gmail.com>
Signed-off-by: Andrew Dye <andrewwdye@gmail.com>
@andrewwdye andrewwdye merged commit fbdb162 into master Sep 27, 2023
14 checks passed
@andrewwdye andrewwdye deleted the k8s-events branch September 27, 2023 23:10
eapolinario pushed a commit that referenced this pull request Sep 28, 2023
* Handle batched TaskExecutionEvent reasons

Signed-off-by: Andrew Dye <andrewwdye@gmail.com>

* Add tests

Signed-off-by: Andrew Dye <andrewwdye@gmail.com>

* Update flyteidl version

Signed-off-by: Andrew Dye <andrewwdye@gmail.com>

* Update to EventReason

Signed-off-by: Andrew Dye <andrewwdye@gmail.com>

---------

Signed-off-by: Andrew Dye <andrewwdye@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants