fix: Rewritten feature-server Helm subchart from Java to Python - #6607
fix: Rewritten feature-server Helm subchart from Java to Python#6607AdityaPatil22 wants to merge 3 commits into
Conversation
| image: | ||
| # image.repository -- Docker image for Feature Server repository | ||
| repository: quay.io/feastdev/feature-server-java | ||
| repository: quay.io/feastdev/feature-server |
There was a problem hiding this comment.
@AdityaPatil22 I think the correct fix is either:
- Remove/deprecate the feast umbrella chart's feature-server subchart entirely (it's stale since Java was dropped from CI in May 2025)
- Rewrite the subchart's deployment.yaml to use feast serve like the standalone feast-feature-server chart does
You can see the Helm chart's deployment template at infra/charts/feast/charts/feature-server/templates/deployment.yaml, it has java -jar command, which won't work with python image.
There was a problem hiding this comment.
Hi @ntkathole,
Thanks for the detailed analysis!
I've updated the subchart's deployment.yaml to use feast serve instead of the legacy java -jar command, aligning it with the standalone feast-feature-server chart.
I also removed the stale Java-specific templates (configmap, secret, and ingress) simplified the values.yaml and README to reflect the current Python-based configuration.
Could you please take a look when you have a chance and let me know if everything looks good, or if there are any additional changes you'd recommend?
4a810a6 to
e155575
Compare
e155575 to
1b19a37
Compare
franciscojavierarceo
left a comment
There was a problem hiding this comment.
The image fix is now bundled with a full Java→Python chart rewrite, so there are two deployment blockers to resolve:
feature_store_yaml_base64is passed through Helm values and directly into the Deployment environment. A normalfeature_store.yamlcommonly contains registry or online-store credentials; putting it in--setvalues stores it in Helm release metadata and exposes it through the Pod spec. The previous chart keptapplication-secret.yamlin a Kubernetes Secret. We should accept a Secret reference or mount the config from a Secret rather than requiring credentials in a value/env string.- Removing the ingress templates, Secret support, and
service.grpcvalues is a breaking change for existing umbrella-chart users, but the PR does not provide a migration path or preserve equivalent ingress/auth configuration. We should either keep those interfaces working for the Python server or document/version the breaking chart migration explicitly.
Signed-off-by: Aditya Patil <[email protected]>
Signed-off-by: Aditya Patil <[email protected]>
Signed-off-by: Aditya Patil <[email protected]>
1b19a37 to
a7703e7
Compare
|
Thanks @franciscojavierarceo, I have addressed both points in the latest push:
|
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #6607 +/- ##
=======================================
Coverage 46.77% 46.77%
=======================================
Files 414 414
Lines 50191 50191
Branches 7181 7181
=======================================
Hits 23475 23475
Misses 25077 25077
Partials 1639 1639
Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
What this PR does / why we need it:
The Helm chart's feature-server subchart references
quay.io/feastdev/feature-server-java, which does not exist in the container registry. Thecorrect image is
quay.io/feastdev/feature-server(without the-javasuffix). This causesImagePullBackOfferrors when deploying Feast via Helm.This PR fixes the image repository name in
infra/charts/feast/charts/feature-server/values.yaml.Which issue(s) this PR fixes:
Fixes #6409
Checks
git commit -s)Testing Strategy
Verified by deploying the chart to a local Kubernetes cluster and confirming pods reach
Runningstate instead ofImagePullBackOff.