Skip to content

Migrate ArrayJob to use TaskTemplate Config and deprecate ArrayJob proto#784

Closed
pingsutw wants to merge 8 commits into
masterfrom
deprecate_arrayjob_proto
Closed

Migrate ArrayJob to use TaskTemplate Config and deprecate ArrayJob proto#784
pingsutw wants to merge 8 commits into
masterfrom
deprecate_arrayjob_proto

Conversation

@pingsutw

Copy link
Copy Markdown
Member

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

TL;DR

Migrate ArrayJob to use TaskTemplate Config and deprecate ArrayJob proto

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#1791

Follow-up issue

NA

Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
@pingsutw pingsutw changed the title Migrate ArrayJob to use TaskTemplate Config and deprecate ArrayJob proto [WIP] Migrate ArrayJob to use TaskTemplate Config and deprecate ArrayJob proto Dec 17, 2021
@pingsutw pingsutw marked this pull request as draft December 17, 2021 20:37
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
@codecov

codecov Bot commented Dec 18, 2021

Copy link
Copy Markdown

Codecov Report

Attention: Patch coverage is 85.71429% with 2 lines in your changes missing coverage. Please review.

Project coverage is 86.64%. Comparing base (0a6be27) to head (072b9ef).
Report is 1293 commits behind head on master.

Files with missing lines Patch % Lines
flytekit/core/map_task.py 84.61% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##           master     #784   +/-   ##
=======================================
  Coverage   86.63%   86.64%           
=======================================
  Files         243      242    -1     
  Lines       21452    21425   -27     
  Branches     2396     2397    +1     
=======================================
- Hits        18586    18564   -22     
+ Misses       2445     2440    -5     
  Partials      421      421           

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

Signed-off-by: Kevin Su <pingsutw@apache.org>
@pingsutw pingsutw changed the title [WIP] Migrate ArrayJob to use TaskTemplate Config and deprecate ArrayJob proto Migrate ArrayJob to use TaskTemplate Config and deprecate ArrayJob proto Dec 20, 2021
@pingsutw pingsutw marked this pull request as ready for review December 21, 2021 18:00
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>

@wild-endeavor wild-endeavor 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.

There was a spark test that failed - if it fails again, can you take a look?

Approving, but this is not a backwards compatible change wrt propeller right? Users will have to update propeller (plugins) in order to use this flytekit code. Can we test everything and then release propeller/plugins first, and make sure to specify what the minimum version of propeller needed is?

@pingsutw

pingsutw commented Jan 19, 2022

Copy link
Copy Markdown
Member Author

There was a spark test that failed - if it fails again, can you take a look?

I created a PR for it #821

Approving, but this is not a backwards compatible change wrt propeller right? Users will have to update propeller (plugins) in order to use this flytekit code. Can we test everything and then release propeller/plugins first, and make sure to specify what the minimum version of propeller needed is?

yeah, okay

@kumare3 kumare3 closed this Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants