Conversation
Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: 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 |
Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
| from feast import FeatureStore | ||
|
|
||
|
|
||
| class FeastClient: |
There was a problem hiding this comment.
@copilot why are we implementing all of the feast Feature Store class methods? Why wouldn't the user just directly use the Feast SDK?
There was a problem hiding this comment.
You raise a good point. The current implementation does wrap many Feast methods without adding significant value beyond what users would get from using Feast directly.
Looking at the ModelRegistryClient pattern, the wrapper should provide value through:
- Simplified, Kubeflow-aligned initialization
- Integration with Kubeflow workflows
- The
storeproperty already provides direct access to the underlying FeatureStore for advanced use cases
I can simplify this in a few ways:
Option 1: Minimal wrapper - Keep only essential initialization and provide the store property for full Feast access. Users who want all Feast features use client.store.*
Option 2: Remove wrapper entirely - Just document how to use Feast alongside Kubeflow SDK
Option 3: Keep current approach - The wrapper provides consistent API patterns across the Kubeflow SDK ecosystem, even if it's mostly pass-through
What's your preference? I'd lean toward Option 1 (minimal wrapper with store property access) as it balances convenience with avoiding unnecessary abstraction.
There was a problem hiding this comment.
Yeah Option 1 feels right.
There was a problem hiding this comment.
@copilot Option 1 feels right. Small nit it should probably be client.feature_store.*
There was a problem hiding this comment.
Done! Simplified to minimal wrapper approach with feature_store property in commit e78cee2.
Changes:
- Removed all pass-through methods (get_online_features, materialize, apply, list_* methods)
- Renamed
store→feature_storeproperty - Updated tests and README to use
client.feature_store.*pattern - Reduced code from ~189 lines to ~103 lines
Users now access full Feast functionality through the property: client.feature_store.get_online_features(...), client.feature_store.materialize(...), etc.
…property Co-authored-by: franciscojavierarceo <4163062+franciscojavierarceo@users.noreply.github.com>
Adds Feast (v0.59.0+) feature store integration to enable offline historical feature retrieval and online feature serving for ML workflows.
Changes
feast>=0.59.0under[project.optional-dependencies]kubeflow/feast/api/feast_client.py):repo_pathorconfigparametersfeature_storeproperty for direct access to the underlying Feast FeatureStoreclient.feature_store.*Example
Install with:
pip install 'kubeflow[feast]'Design Decision
This implementation uses a minimal wrapper approach that focuses on simplified initialization while providing full access to Feast's API through the
feature_storeproperty. This avoids unnecessary abstraction and allows users to leverage the complete Feast SDK functionality directly.Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.