FLYTE_INTERNAL_IMAGE should have higher precedence #2523
Conversation
Signed-off-by: Kevin Su <pingsutw@apache.org>
Signed-off-by: Kevin Su <pingsutw@apache.org>
pingsutw
requested review from
wild-endeavor,
kumare3,
eapolinario,
cosmicBboy and
samhita-alla
as code owners
|
cc @thomasjpfan |
|
|
||
| @classmethod | ||
| def find_image_for( | ||
| cls, python_version: typing.Optional[PythonVersion] = None, flytekit_version: typing.Optional[str] = None | ||
| ) -> str: | ||
| default_image_str = os.getenv("FLYTE_INTERNAL_IMAGE") |
There was a problem hiding this comment.
FLYTE_INTERNAL_IMAGE is not really documented. What is it used for?
There was a problem hiding this comment.
It's very useful when I need to override the default image for the task or the base image in the ImageSpec.
There was a problem hiding this comment.
can we move FLYTE_INTERNAL_IMAGE to a constant and leave this comment there?
Co-authored-by: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2523 +/- ##
==========================================
+ Coverage 71.79% 76.15% +4.35%
==========================================
Files 182 182
Lines 18561 18565 +4
Branches 3654 3655 +1
==========================================
+ Hits 13326 14138 +812
+ Misses 4592 3800 -792
+ Partials 643 627 -16 ☔ View full report in Codecov by Sentry. |
Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>
Signed-off-by: Kevin Su <pingsutw@apache.org> Co-authored-by: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com> Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> Signed-off-by: bugra.gedik <bugra.gedik@predera.ai>
Signed-off-by: Kevin Su <pingsutw@apache.org> Co-authored-by: Eduardo Apolinario <653394+eapolinario@users.noreply.github.com> Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com> Signed-off-by: Jan Fiedler <jan@union.ai>
Tracking issue
NA
Why are the changes needed?
When unionai is installed, it will ignore
FLYTE_INTERNAL_IMAGE, and always use the default flytekit image.That's because flytekit will call
get_plugin().get_default_image(), and thenDefaultImages.find_image_for(). Thus, it skip the FLYTE_INTERNAL_IMAGE.What changes were proposed in this pull request?
Move
os.getenv("FLYTE_INTERNAL_IMAGE")tofind_image_for()How was this patch tested?
unit test
Setup process
Screenshots
Check all the applicable boxes
Related PRs
NA
Docs link
NA