Conversation
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
tsotnet
requested review from
davidheryanto,
khorshuheng,
pyalex and
zhilingc
as code owners
infra/charts/feast/charts/feast-jobservice/templates/deployment.yaml
Outdated
Show resolved
Hide resolved
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
|
@oavdeev thanks for the comments, I realized most of the clean up stuff and was cleaning up the diff. Didn't find an equivalent functionality of Phabricator's "Plan Changes" to not notify reviewers... Let me know if there's anything else left! |
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
tsotnet
force-pushed
the
job-service-helm
branch
from
627afa4 to
4d7c9e4
Compare
| {{- toYaml . | nindent 8 }} | ||
| {{- end }} | ||
|
|
||
| {{- if .Values.gcpServiceAccount.enabled }} |
There was a problem hiding this comment.
Do we need to hardcode the GCP specific fields here still? I think we want to move away from this (if we support AWS)
There was a problem hiding this comment.
@woop we should probably handle that in another PR to get this one unblocked.
- All services (core, serving, jupyter) have the gcp-specific fields, so we need to figure out the overall solution here, not just for job service
- Looks like for AWS @oavdeev wrote the terraform script that uses envOverrides to override some environment variables, and he won't need to hardcode any AWS-specific stuff. However, this approach can't be used readily, since we don't have terraform for GCP deployment & some of these gcp fields are used outside env variables right now. Therefore I don't have a clear alternative to move these variables to.
- It won't affect AWS deployment in the short term, so we can temporarily leave it here
Let me know what you think!
There was a problem hiding this comment.
Sure, lets go ahead with cleaning this up in another PR.
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
tsotnet
force-pushed
the
job-service-helm
branch
from
4e9310b to
d6adf2e
Compare
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
tsotnet
force-pushed
the
job-service-helm
branch
from
3b78bc8 to
b3aca8d
Compare
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: oavdeev, tsotnet The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
/retest |
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
tsotnet
force-pushed
the
job-service-helm
branch
from
0532ae1 to
e6c0887
Compare
Signed-off-by: Tsotne Tabidze <tsotnet@gmail.com>
tsotnet
force-pushed
the
job-service-helm
branch
from
a8336bd to
5ad7a3d
Compare
|
/retest |
1 similar comment
|
/retest |
4 participants
What this PR does / why we need it: This PR adds Helm Chart for Feast Job Service. Tested locally on minikube.