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

Add Raw AWS Batch Task#228

Merged
pingsutw merged 21 commits into
masterfrom
aws-batch-1
Feb 7, 2022
Merged

Add Raw AWS Batch Task#228
pingsutw merged 21 commits into
masterfrom
aws-batch-1

Conversation

@pingsutw

@pingsutw pingsutw commented Dec 17, 2021

Copy link
Copy Markdown
Member

Signed-off-by: Kevin Su pingsutw@apache.org

TL;DR

Run regular tasks on AWS batch service
check example in flyteorg/flytekit#782

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

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

flyteorg/flyte#1792
flyteorg/flytekit#782

Follow-up issue

NA

Signed-off-by: Kevin Su <pingsutw@apache.org>
@pingsutw pingsutw changed the title Add Raw AWS Batch Task [WIP]Add Raw AWS Batch Task Dec 17, 2021
@pingsutw pingsutw changed the title [WIP]Add Raw AWS Batch Task [WIP] Add Raw AWS Batch Task Dec 17, 2021
Signed-off-by: Kevin Su <pingsutw@apache.org>
@codecov

codecov Bot commented Dec 21, 2021

Copy link
Copy Markdown

Codecov Report

Merging #228 (1b8cecb) into master (973a51a) will increase coverage by 0.16%.
The diff coverage is 79.62%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #228      +/-   ##
==========================================
+ Coverage   62.30%   62.47%   +0.16%     
==========================================
  Files         141      142       +1     
  Lines        8807     8868      +61     
==========================================
+ Hits         5487     5540      +53     
- Misses       2821     2822       +1     
- Partials      499      506       +7     
Flag Coverage Δ
unittests 62.07% <80.76%> (+0.16%) ⬆️

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

Impacted Files Coverage Δ
...plugins/array/awsbatch/definition/job_def_cache.go 0.00% <0.00%> (ø)
go/tasks/plugins/array/awsbatch/transformer.go 77.39% <75.00%> (-1.48%) ⬇️
go/tasks/plugins/array/outputs.go 72.18% <75.00%> (-0.99%) ⬇️
go/tasks/plugins/array/awsbatch/client.go 64.06% <100.00%> (+0.57%) ⬆️
go/tasks/plugins/array/awsbatch/executor.go 40.56% <100.00%> (ø)
go/tasks/plugins/array/awsbatch/job_definition.go 69.23% <100.00%> (+2.56%) ⬆️
go/tasks/plugins/array/catalog.go 48.58% <100.00%> (+1.69%) ⬆️
go/tasks/plugins/array/inputs.go 83.33% <100.00%> (ø)
go/tasks/plugins/array/awsbatch/launcher.go 57.89% <0.00%> (-1.73%) ⬇️
go/tasks/plugins/webapi/athena/plugin.go 17.85% <0.00%> (-1.45%) ⬇️
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 973a51a...1b8cecb. Read the comment docs.

Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
@pingsutw pingsutw changed the title [WIP] Add Raw AWS Batch Task Add Raw AWS Batch Task Dec 22, 2021
Signed-off-by: Kevin Su <pingsutw@apache.org>
Comment thread go/tasks/plugins/array/awsbatch/client.go Outdated
@kumare3 kumare3 requested review from EngHabu and hamersaw January 19, 2022 00:20
@kumare3

kumare3 commented Jan 19, 2022

Copy link
Copy Markdown
Contributor

mostly looks good to me, but, looks a little more complicated because of the array case

Signed-off-by: Kevin Su <pingsutw@apache.org>

@hamersaw hamersaw left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few questions regarding implementation and a couple nits. I think it looks great!

Comment thread go/tasks/plugins/array/awsbatch/definition/job_def_cache.go Outdated
Comment thread go/tasks/plugins/array/catalog.go Outdated
Comment thread go/tasks/plugins/array/awsbatch/client.go Outdated
Comment thread go/tasks/plugins/array/outputs.go Outdated
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Comment thread go/tasks/plugins/array/awsbatch/definition/job_def_cache.go Outdated
Comment thread go/tasks/plugins/array/awsbatch/definition/job_def_cache.go Outdated
Signed-off-by: Kevin Su <pingsutw@apache.org>
Comment thread go/tasks/plugins/array/awsbatch/client.go Outdated
Comment thread go/tasks/plugins/array/awsbatch/client.go Outdated
Comment thread go/tasks/plugins/array/outputs.go Outdated
Comment thread go/tasks/plugins/array/awsbatch/definition/job_def_cache.go Outdated
Comment thread go/tasks/plugins/array/awsbatch/job_definition.go Outdated
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
@pingsutw pingsutw requested a review from EngHabu January 24, 2022 18:30
Comment thread go/tasks/plugins/array/awsbatch/definition/job_def_cache.go Outdated
Comment thread go/tasks/plugins/array/awsbatch/job_definition.go Outdated
Comment thread go/tasks/plugins/array/awsbatch/transformer.go
Signed-off-by: Kevin Su <pingsutw@apache.org>

@EngHabu EngHabu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @pingsutw almost there... I've a couple more comments!

Comment thread go/tasks/plugins/array/awsbatch/job_definition.go Outdated
Comment thread go/tasks/plugins/array/awsbatch/transformer.go
Signed-off-by: Kevin Su <pingsutw@apache.org>

@EngHabu EngHabu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @pingsutw

@pingsutw pingsutw merged commit ac44eae into master Feb 7, 2022
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
* Add Raw AWS Batch Task

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Fix test

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Fix lint

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Fix lint

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Add tests

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Fix lint

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Remove log

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Updated tests and added comment

Signed-off-by: Kevin Su <pingsutw@apache.org>

* address comment

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Fix tests

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Convert pb to string

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Use job definition as cache key

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Fixed tests

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Fixed tests

Signed-off-by: Kevin Su <pingsutw@apache.org>

* One more test

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Hash job definition

Signed-off-by: Kevin Su <pingsutw@apache.org>

* address comments

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Updated dependency

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Fixed test

Signed-off-by: Kevin Su <pingsutw@apache.org>

* lint fixed

Signed-off-by: Kevin Su <pingsutw@apache.org>

* Reorder

Signed-off-by: Kevin Su <pingsutw@apache.org>
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.

4 participants