Skip to content

Conversation

adchia
Copy link
Collaborator

@adchia adchia commented Feb 10, 2022

What this PR does / why we need it:
There were some out of date fields in the helm charts, and overriding them would cause breakages. Project names and registry paths were also not being passed to the feature server.

This also adds a tutorial on how to get a Java server + Redis + GCS based registry working.

Which issue(s) this PR fixes:

Fixes #

Does this PR introduce a user-facing change?:

Tutorial for using Java feature server with Helm

Signed-off-by: Danny Chiao <[email protected]>
Signed-off-by: Danny Chiao <[email protected]>
Signed-off-by: Danny Chiao <[email protected]>
Signed-off-by: Danny Chiao <[email protected]>
Signed-off-by: Danny Chiao <[email protected]>
Signed-off-by: Danny Chiao <[email protected]>
Signed-off-by: Danny Chiao <[email protected]>
@adchia adchia requested a review from a team as a code owner February 10, 2022 00:36
@adchia adchia requested review from kevjumba and removed request for a team February 10, 2022 00:36
Signed-off-by: Danny Chiao <[email protected]>
Signed-off-by: Danny Chiao <[email protected]>
Signed-off-by: Danny Chiao <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented Feb 10, 2022

Codecov Report

Merging #2298 (81759f9) into master (144f25c) will decrease coverage by 0.09%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2298      +/-   ##
==========================================
- Coverage   86.15%   86.05%   -0.10%     
==========================================
  Files         116      116              
  Lines        9820     9820              
==========================================
- Hits         8460     8451       -9     
- Misses       1360     1369       +9     
Flag Coverage Δ
integrationtests 74.77% <ø> (-0.38%) ⬇️
unittests 58.55% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
sdk/python/feast/wait.py 58.82% <0.00%> (-35.30%) ⬇️
.../integration/online_store/test_universal_online.py 96.77% <0.00%> (-1.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 144f25c...81759f9. Read the comment docs.

Signed-off-by: Danny Chiao <[email protected]>
Signed-off-by: Danny Chiao <[email protected]>
Signed-off-by: Danny Chiao <[email protected]>
@woop
Copy link
Member

woop commented Feb 14, 2022

@adchia do you need a review here?

@adchia
Copy link
Collaborator Author

adchia commented Feb 14, 2022

yeah a review would be useful, but i think merging it in as is is probably fine since folks have successfully started using this new helm chart again

Copy link
Collaborator

@felixwang9817 felixwang9817 left a comment

Choose a reason for hiding this comment

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

/lgtm

@feast-ci-bot
Copy link
Collaborator

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: adchia, felixwang9817

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [adchia,felixwang9817]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@feast-ci-bot feast-ci-bot merged commit bd93766 into feast-dev:master Feb 14, 2022
@adchia adchia deleted the fixHelmJava branch February 14, 2022 18:42
adchia added a commit that referenced this pull request Feb 15, 2022
…them (#2298)

* Fixing helm chart and adding java tutorial

Signed-off-by: Danny Chiao <[email protected]>

* fix chart version

Signed-off-by: Danny Chiao <[email protected]>

* test

Signed-off-by: Danny Chiao <[email protected]>

* Parse the overrides to not pass enabled

Signed-off-by: Danny Chiao <[email protected]>

* Disable secrets by default

Signed-off-by: Danny Chiao <[email protected]>

* Change secrets default

Signed-off-by: Danny Chiao <[email protected]>

* revert

Signed-off-by: Danny Chiao <[email protected]>

* Update README

Signed-off-by: Danny Chiao <[email protected]>

* finally

Signed-off-by: Danny Chiao <[email protected]>

* remove credentials

Signed-off-by: Danny Chiao <[email protected]>

* new line

Signed-off-by: Danny Chiao <[email protected]>

* new line

Signed-off-by: Danny Chiao <[email protected]>

* new line

Signed-off-by: Danny Chiao <[email protected]>

* Add reference from Gitbook

Signed-off-by: Danny Chiao <[email protected]>

* remove description

Signed-off-by: Danny Chiao <[email protected]>
aht added a commit to aht/feast that referenced this pull request Feb 23, 2022
…ents

* upstream/master: (103 commits)
  refactor: Move create cluster script
  ci: Add validation for upper case PR titles
  Switch order of views to keep/delete to fix apply logging
  ci: Fix broken Prow plugin which ignores config.yaml
  ci: Add PR linting to ensure conventional commits are followed for PR titles
  Add owner field to FeatureService (feast-dev#2321)
  Disable Redis cluster tests (feast-dev#2327)
  Use PR title for commit message
  Add redis cluster initialziation to master github workflow (feast-dev#2317)
  Fix materialize bug with RedisCluster (feast-dev#2311)
  Add support for Dask during historical retrieval (feast-dev#1954)
  downgrade datatypes to 8
  Update pom.xml to 0.18.2 SNAPSHOT
  Update changelog for 0.18.1 and update pom.xml
  Update helm chart version
  Don't require `snowflake` to always be installed (feast-dev#2309)
  Fixing the Java helm charts and adding a demo tutorial on how to use them (feast-dev#2298)
  Include infra objects in registry dump and fix Infra's from_proto  (feast-dev#2295)
  Remove old flag warning with the python feature server (feast-dev#2300)
  podAnnotations Values in the feature-server chart (feast-dev#2304)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants