FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: Operator - make server container creation explicit in the CR by tchughesiv · Pull Request #5024 · feast-dev/feast · GitHub

/ feast Public

feat: Operator - make server container creation explicit in the CR - #5024

Merged
tchughesiv merged 1 commit into
feast-dev:masterfrom
tchughesiv:serverOpt
Feb 7, 2025
ConversationCommits (1)ChecksFiles changed
Merged

feat: Operator - make server container creation explicit in the CR#5024
tchughesiv merged 1 commit into
feast-dev:masterfrom
tchughesiv:serverOpt

Conversation

tchughesiv commented Feb 6, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

What this PR does / why we need it:

Currently, the operator creates a running server/container for each feast service by default. This is unnecessary and should only be done on an as-needed basis. With this change, the user will have to explicitly configure a new server subsection for a given service type (online/offline/etc). Otherwise, the operator will not run a separate container w/ remote server for that service. By default, the registry container will continue created.

Which issue(s) this PR fixes:

Fixes #5023

Misc

In the following example the online store, but NOT the offline, would have a separate container running a remote online feature server. The offline persistence configs, however, would still be reflected in the shared feature_store.yaml.

apiVersion: feast.dev/v1alpha1
kind: FeatureStore
metadata:
  name: example
spec:
  feastProject: my_project
  services:
    offlineStore:
      persistence:
        file:
          type: duckdb
    onlineStore:
      server: {}

an example of some of the optional fields within this server object -

      server:
        image: 'feastdev/feature-server:0.45.0'
        logLevel: debug
        tls:
          secretKeyNames:
            tlsCrt: tls.crt
            tlsKey: tls.key
          secretRef:
            name: feast-sample-all-servers-online-tls
        envFrom:
          - secretRef:
              name: postgres-secret
        resources:
          requests:
            cpu: 150m
            memory: 128Mi

tchughesiv requested a review from a team as a code owner February 6, 2025 21:54
tchughesiv marked this pull request as draft February 6, 2025 21:54
tchughesiv changed the title feat: Operator- Make server container creation explicit in the CR feat: Operator - make server container creation explicit in the CR Feb 6, 2025
add server option
eb5c7e8 
Signed-off-by: Tommy Hughes <tohughes@redhat.com>
tchughesiv marked this pull request as ready for review February 6, 2025 22:43

redhatHameed commented Feb 7, 2025 •
edited
Loading

Copy link
Copy Markdown
Contributor

@tchughesiv WDYT
if we can just flag remote it will be be false for offline server, for online always true

example

apiVersion: feast.dev/v1alpha1
kind: FeatureStore
metadata:
  name: sample-all-services
spec:
  feastProject: my_project
  services:
    onlineStore:
      remote: true
    offlineStore:
      remote: false
    registry:
      remote:?

if this is just for offline server setting we just go with this settings

apiVersion: feast.dev/v1alpha1
kind: FeatureStore
metadata:
  name: sample-all-services
spec:
  feastProject: my_project
  services:
    onlineStore {}
    offlineStore:
      remote: false
    registry:
      local: {}

tmihalac reviewed Feb 7, 2025
View reviewed changes
Comment thread infra/feast-operator/internal/controller/featurestore_controller_loglevel_test.go
})
})

func strPtr(str string) *string {

tmihalac Feb 7, 2025

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Are you sure a one line function like this is needed?

tchughesiv Feb 7, 2025

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

unfortunately yes... the alternative would be separate vars for each.

redhatHameed commented Feb 7, 2025

Copy link
Copy Markdown
Contributor

@tchughesiv WDYT if we can just flag remote it will be be false for offline server, for online always true

example

apiVersion: feast.dev/v1alpha1
kind: FeatureStore
metadata:
  name: sample-all-services
spec:
  feastProject: my_project
  services:
    onlineStore:
      remote: true
    offlineStore:
      remote: false
    registry:
      remote:?

if this is just for offline server setting we just go with this settings

apiVersion: feast.dev/v1alpha1
kind: FeatureStore
metadata:
  name: sample-all-services
spec:
  feastProject: my_project
  services:
    onlineStore {}
    offlineStore:
      remote: false
    registry:
      local: {}

Thanks, @tchughesiv for explaining on this on meet/call, I am good to keep current approach using sever section

redhatHameed approved these changes Feb 7, 2025
View reviewed changes

redhatHameed left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

lgtm

tchughesiv merged commit b16fb40 into feast-dev:master Feb 7, 2025
franciscojavierarceo pushed a commit that referenced this pull request Feb 17, 2025
chore(release): release 0.46.0
6372f1c 
# [0.46.0](v0.45.0...v0.46.0) (2025-02-17)

### Bug Fixes

* Add scylladb to online stores list in docs ([#5061](#5061)) ([08183ed](08183ed))
* Changed feast operator to set status of featurestore cr to ready based on deployment.status = available ([#5020](#5020)) ([fce0d35](fce0d35))
* Ensure Postgres queries are committed or autocommit is used ([#5039](#5039)) ([46f8d7a](46f8d7a))
* Fixing the release workflow to refresh the stable branch when the release is not running in the dry run mode. ([#5057](#5057)) ([a13fa9b](a13fa9b))
* Operator - make onlineStore the default service ([#5044](#5044)) ([6c92447](6c92447))
* Operator - resolve infinite reconciler loop in authz controller ([#5056](#5056)) ([11e4548](11e4548))
* Resolve module on windows ([#4827](#4827)) ([efbffa4](efbffa4))
* Setting the github_token explicitly to see if that solves the problem. ([#5012](#5012)) ([3834ffa](3834ffa))
* Validate entities when running get_online_features ([#5031](#5031)) ([3bb0dca](3bb0dca))

### Features

* Add SQLite retrieve_online_documents_v2 ([#5032](#5032)) ([0fffe21](0fffe21))
* Adding Click command to display configuration details ([#5036](#5036)) ([ae68e4d](ae68e4d))
* Adding volumes and volumeMounts support to Feature Store CR. ([#4983](#4983)) ([ec6f1b7](ec6f1b7))
* Moving the job to seperate action so that we can test it easily. ([#5013](#5013)) ([b9325b7](b9325b7))
* Operator - make server container creation explicit in the CR ([#5024](#5024)) ([b16fb40](b16fb40))
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

redhatHameed redhatHameed approved these changes
+1 more reviewer
tmihalac tmihalac left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

ok-to-test

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Operator should only create online/offline remote server containers when necessary

3 participants

Back | FazBrowse Home | New Git URL