Skip to content

Conversation

yunfeng-scale
Copy link
Contributor

@yunfeng-scale yunfeng-scale commented Nov 16, 2023

Pull Request Summary

Pull policy for these images should be IfNotPresent, otherwise minikube wouldn't be able to load them and fine tune jobs can't start. i'm not sure how finetune integration tests have been passing (maybe we actually pulled the image from minikube?)

Test Plan and Usage Guide

How did you validate that your PR works correctly? How do you run or demo the code? Provide enough detail so a reviewer can reasonably reproduce the testing procedure. Paste example command line invocations if applicable.

@yunfeng-scale yunfeng-scale requested review from a team, seanshi-scale and tiffzhao5 November 16, 2023 05:38
{{- end }}
{{- end }}
imagePullPolicy: Always
imagePullPolicy: IfNotPresent
Copy link
Contributor Author

@yunfeng-scale yunfeng-scale Nov 16, 2023

Choose a reason for hiding this comment

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

@seanshi-scale let me know if this change is okay

Copy link
Contributor

Choose a reason for hiding this comment

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

This should be fine, think our image tags are immutable

@seanshi-scale
Copy link
Contributor

iirc for the endpoints we explicitly load images into minikube at some point, but I'm not sure if the imagePullPolicy on the endpoints is the same

@yunfeng-scale
Copy link
Contributor Author

yunfeng-scale commented Nov 16, 2023

iirc for the endpoints we explicitly load images into minikube at some point, but I'm not sure if the imagePullPolicy on the endpoints is the same

i'm seeing Always on the ft_ jobs and failed to pull images and thought this should fix

@yunfeng-scale yunfeng-scale merged commit 2221de0 into main Nov 16, 2023
@yunfeng-scale yunfeng-scale deleted the yunfeng-integration-tet branch November 16, 2023 18:12
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.

2 participants