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

Add env FLYTE_FAIL_ON_ERROR to aws batch job #306

Merged
merged 8 commits into from
Jan 23, 2023
Merged

Add env FLYTE_FAIL_ON_ERROR to aws batch job #306

merged 8 commits into from
Jan 23, 2023

Conversation

pingsutw
Copy link
Member

@pingsutw pingsutw commented Jan 6, 2023

Signed-off-by: Kevin Su [email protected]

TL;DR

Add env FLYTE_FAIL_ON_ERROR to aws batch job, so the flytekit will return error code once the job fail.

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

image

Tracking Issue

https://flyte-org.slack.com/archives/C01P3B761A6/p1664972219043289

Follow-up issue

NA

@codecov
Copy link

codecov bot commented Jan 6, 2023

Codecov Report

Merging #306 (ff6e9e9) into master (109224c) will increase coverage by 1.33%.
The diff coverage is 100.00%.

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

@@            Coverage Diff             @@
##           master     #306      +/-   ##
==========================================
+ Coverage   63.02%   64.35%   +1.33%     
==========================================
  Files         148      148              
  Lines       12148     9859    -2289     
==========================================
- Hits         7656     6345    -1311     
+ Misses       3912     2934     -978     
  Partials      580      580              
Flag Coverage Δ
unittests ?

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

Impacted Files Coverage Δ
go/tasks/plugins/array/awsbatch/transformer.go 82.31% <100.00%> (+3.30%) ⬆️
go/tasks/plugins/array/inputs.go 83.33% <0.00%> (-4.91%) ⬇️
.../pluginmachinery/internal/webapi/plugin_context.go 50.00% <0.00%> (-3.85%) ⬇️
go/tasks/plugins/k8s/sagemaker/plugin.go 74.07% <0.00%> (-3.20%) ⬇️
go/tasks/plugins/array/core/state.go 69.13% <0.00%> (-2.85%) ⬇️
go/tasks/pluginmachinery/flytek8s/pod_helper.go 81.43% <0.00%> (-2.64%) ⬇️
go/tasks/plugins/hive/config/config.go 33.33% <0.00%> (-2.39%) ⬇️
go/tasks/plugins/presto/config/config.go 33.33% <0.00%> (-2.39%) ⬇️
go/tasks/plugins/array/core/metadata.go 80.00% <0.00%> (-2.23%) ⬇️
go/tasks/pluginmachinery/catalog/response.go 42.85% <0.00%> (-2.15%) ⬇️
... and 123 more

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

@pingsutw pingsutw requested a review from hamersaw January 7, 2023 00:35
@hamersaw
Copy link
Contributor

hamersaw commented Jan 9, 2023

This seems very hacky. We should probably dive into this a bit further and see how we can have subtask phases match between AWS batch and Flyte reporting.

@pingsutw pingsutw changed the title Correct aws batch job state Add env FLYTE_FAIL_ON_ERROR to aws batch job Jan 11, 2023
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
@pingsutw pingsutw merged commit 4634a81 into master Jan 23, 2023
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
* Correct aws batch job state

Signed-off-by: Kevin Su <[email protected]>

* Add env FAST_ON_ERROR to aws batch job

Signed-off-by: Kevin Su <[email protected]>

* Add tests

Signed-off-by: Kevin Su <[email protected]>

* update tests

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

* update test

Signed-off-by: Kevin Su <[email protected]>

* nit

Signed-off-by: Kevin Su <[email protected]>

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

2 participants