8000 Handle batched TaskExecutionEvent reasons by andrewwdye · Pull Request #615 · flyteorg/flyteadmin · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content
This repository was 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
8000 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

@andrewwdye
Handle batched TaskExecutionEvent reasons
b39b631 
Signed-off-by: Andrew Dye <andrewwdye@gmail.com>
@andrewwdye
Merge branch 'master' of https://github.com/flyteorg/flyteadmin into …
a0b97d5 
…k8s-events

Signed-off-by: Andrew Dye <andrewwdye@gmail.com>
@andrewwdye
Add tests
dfa2db6 
Signed-off-by: Andrew Dye <andrewwdye@gmail.com>
@andrewwdye andrewwdye marked this pull request as ready for review September 26, 2023 00:41
@codecov
Copy link
codecov bot commented Sep 26, 2023 •
edited
Loading

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.

katrogan
katrogan previously approved these changes Sep 26, 2023
View reviewed changes
@andrewwdye
Update flyteidl version
fd50f54 
Signed-off-by: Andrew Dye <andrewwdye@gmail.com>
@andrewwdye
Update to EventReason
c049ced 
Signed-off-by: Andrew Dye <andrewwdye@gmail.com>
@andrewwdye andrewwdye merged commit fbdb162 into master Sep 27, 2023
@andrewwdye andrewwdye deleted the k8s-events branch September 27, 2023 23:10
eapolinario pushed a commit that referenced this pull request Sep 28, 2023
@andrewwdye
Handle batched TaskExecutionEvent reasons (#615)
67e65c7 
* 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.
Reviewers

@katrogan katrogan katrogan approved these changes

@wild-endeavor wild-endeavor Awaiting requested review from wild-endeavor wild-endeavor is a code owner

@EngHabu EngHabu Awaiting requested review from EngHabu EngHabu is a code owner

@kumare3 kumare3 Awaiting requested review from kumare3 kumare3 is a code owner

@pmahindrakar-oss pmahindrakar-oss Awaiting requested review from pmahindrakar-oss pmahindrakar-oss is a code owner

@hamersaw hamersaw Awaiting requested review from hamersaw hamersaw is a code owner

@eapolinario eapolinario Awaiting requested review from eapolinario eapolinario is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@andrewwdye @katrogan
0