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

Added template configuration to task template #358

Merged
merged 5 commits into from
Jan 30, 2023

Conversation

hamersaw
Copy link
Contributor

TL;DR

This PR adds a template field to the TaskTemplate which contains either a PodTemplate name or a marshaled PodTemplate struct.

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

flyteorg/flyte#3123

Follow-up issue

NA

@codecov
Copy link

codecov bot commented Jan 11, 2023

Codecov Report

Merging #358 (1e96e6a) into master (c1c892a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #358   +/-   ##
=======================================
  Coverage   73.71%   73.71%           
=======================================
  Files          18       18           
  Lines        1377     1377           
=======================================
  Hits         1015     1015           
  Misses        311      311           
  Partials       51       51           
Flag Coverage Δ
unittests 73.71% <ø> (ø)

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

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

Copy link
Contributor

@EngHabu EngHabu left a comment

Choose a reason for hiding this comment

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

Let's add more code comments to explain the behavior. I'm pretty sure we will get a lot of questions here...

protos/flyteidl/core/tasks.proto Outdated Show resolved Hide resolved
@kumare3
Copy link
Contributor

kumare3 commented Jan 14, 2023

Why do we need this, why not use the one of

@flixr
Copy link

flixr commented Jan 19, 2023

@kumare3 e.g. for my usecases it boils down to setting some things like runtimeClassName, affinity, etc. per task (some tasks in the workflow need to run on nodes with large GPU, same only need a small one). And it would allow mounting of secrets, etc.. see also flyteorg/flyte#3055

@hamersaw hamersaw requested a review from EngHabu January 20, 2023 17:00
@hamersaw hamersaw merged commit c60305c into master Jan 30, 2023
@hamersaw hamersaw deleted the feature/task-pod-template branch January 30, 2023 15:44
eapolinario pushed a commit that referenced this pull request Sep 8, 2023
* added template to task template

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

* updated docs

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

* updated pod_template_name location to TaskMetadata proto message

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

---------

Signed-off-by: Daniel 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.

5 participants