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

sending correct external resources for k8s-array plugin #300

Merged
merged 8 commits into from
Feb 8, 2023

Conversation

hamersaw
Copy link
Contributor

@hamersaw hamersaw commented Dec 2, 2022

TL;DR

Cache status of map tasks subtasks is not reported correctly. This PR fixes the issue, where all external resources may be correctly annotated with the correct cache status during execution.

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

^^^

Tracking Issue

NA

Follow-up issue

NA

@hamersaw hamersaw marked this pull request as ready for review December 13, 2022 02:55
@codecov
Copy link

codecov bot commented Dec 13, 2022

Codecov Report

Merging #300 (a12e060) into master (d5295d2) will decrease coverage by 0.04%.
The diff coverage is 39.70%.

❗ Current head a12e060 differs from pull request most recent head 52ed4d2. Consider uploading reports for the commit 52ed4d2 to get more accurate results

@@            Coverage Diff             @@
##           master     #300      +/-   ##
==========================================
- Coverage   62.27%   62.23%   -0.05%     
==========================================
  Files         145      145              
  Lines       11511    11530      +19     
==========================================
+ Hits         7169     7176       +7     
- Misses       3797     3811      +14     
+ Partials      545      543       -2     
Flag Coverage Δ
unittests 62.23% <39.70%> (-0.05%) ⬇️

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

Impacted Files Coverage Δ
go/tasks/plugins/array/catalog.go 45.04% <0.00%> (-2.11%) ⬇️
go/tasks/plugins/array/awsbatch/executor.go 42.55% <33.33%> (+5.59%) ⬆️
go/tasks/plugins/array/k8s/executor.go 39.58% <87.50%> (+0.85%) ⬆️
go/tasks/plugins/array/awsbatch/job_definition.go 68.62% <100.00%> (ø)
go/tasks/plugins/array/awsbatch/launcher.go 59.09% <100.00%> (ø)
go/tasks/plugins/array/awsbatch/monitor.go 67.42% <100.00%> (ø)
go/tasks/plugins/array/core/state.go 71.34% <100.00%> (-0.63%) ⬇️
go/tasks/plugins/array/k8s/management.go 58.33% <100.00%> (ø)
... and 1 more

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

go/tasks/plugins/array/catalog.go Show resolved Hide resolved
go/tasks/plugins/array/core/state.go Outdated Show resolved Hide resolved
@hamersaw hamersaw requested a review from EngHabu December 15, 2022 15:51
@cosmicBboy
Copy link

@EngHabu friendly ping: does this PR look good to go? Currently blocking flyteorg/flyte#3315

@hamersaw hamersaw merged commit e79acfb into master Feb 8, 2023
@hamersaw hamersaw deleted the bug/map-task-cache-status branch February 8, 2023 23:55
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
* sending correct external resources for k8s-array plugin

Signed-off-by: Dan Rammer <[email protected]>

* bumping phase versions

Signed-off-by: Dan Rammer <[email protected]>

* using externalResources in aws-batch plugin

Signed-off-by: Dan Rammer <[email protected]>

* lint issues

Signed-off-by: Dan Rammer <[email protected]>

* fixed unit tests

Signed-off-by: Dan Rammer <[email protected]>

* removed unnecessary comments

Signed-off-by: Dan Rammer <[email protected]>

* reverted permanent failure computation

Signed-off-by: Dan Rammer <[email protected]>

---------

Signed-off-by: Dan Rammer <[email protected]>
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.

3 participants