feat: Update the go feature server from Expedia code repo.#4665
feat: Update the go feature server from Expedia code repo.#4665franciscojavierarceo merged 10 commits intofeast-dev:masterfrom
Conversation
|
cc: @EXPEbdodla |
80285e6 to
d371247
Compare
|
[Update 10-22-2024] Now it can compile, build and build docker image. All configures in the Makefile under the "#Go SDK & embedded" are working, except the "install-go-ci-dependencies". The current "install-go-ci-dependencies" is obsoleted. We need to fix it when we setup the integration test for Go feature server. !!Warning, the current version my not function correctly. need more testing and investigating. |
9ee4605 to
c5aaf98
Compare
|
[Update 10-31-2024]
|
go/internal/feast/registry/http.go
Outdated
| @@ -0,0 +1,212 @@ | |||
| package registry | |||
There was a problem hiding this comment.
This needs to be switched to remote registry. We built http registry to overcome the password sharing for SQL Registry. HTTP Registry which is FastAPI based is not contributed to Feast. We are also planning to leverage Remote registry and move over from HTTP one.
There was a problem hiding this comment.
Thank you for the info, @EXPEbdodla ! Let me try to resolve this.
There was a problem hiding this comment.
I have removed the code of HTTP Registry. And now, only the File based Registry is supported.
|
[Update 11/04/2024]
|
go/internal/feast/featurestore.go
Outdated
| } | ||
| sanitizedProjectName := strings.Replace(config.Project, "_", "-", -1) | ||
| productName := os.Getenv("PRODUCT") | ||
| endpoint := fmt.Sprintf("%s-transformations.%s.svc.cluster.local:80", sanitizedProjectName, productName) |
There was a problem hiding this comment.
Is this an external Transformation server? I guess we need to remove this and related code? @EXPEbdodla
There was a problem hiding this comment.
Initially transformations are running with GOPY bindings. That's not working at scale. So we implemented it similar to Java Feature Server.
There was a problem hiding this comment.
Thank you, @EXPEbdodla I guess I need to work on this part.
There was a problem hiding this comment.
I chose an "easy" fix for this. 😿
There was a problem hiding this comment.
Yes. We should get the transformation server endpoint from feature_store.yaml.
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
…ntation code. Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
…point. Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
b65a6a4 to
953d62e
Compare
|
[Update 11-20-2024] |
| requestedFeatureViewNames []string, | ||
| requestedFeatureNames []string, | ||
| ) ([][]onlinestore.FeatureData, error) { | ||
| // Create a Datadog span from context |
There was a problem hiding this comment.
I think these can be removed or updated as datadog is not part of feast
There was a problem hiding this comment.
Yea. Datadog needs to be replaced with OTEL.
There was a problem hiding this comment.
yes, I will replace them with OTEL after this PR get merged.
| } | ||
|
|
||
| if t == redisNode { | ||
| // Metrics are not showing up when the service name is set to DD_SERVICE |
| @@ -1,109 +1,12 @@ | |||
| This directory contains the Go logic that's executed by the `EmbeddedOnlineFeatureServer` from Python. | |||
There was a problem hiding this comment.
Any reason we are removing the document here?
There was a problem hiding this comment.
That piece of code is not needed if we remove GOPY bindings.
…re.yaml file instead of OS env. Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
|
[Update 11/24/2024]
@EXPEbdodla @feast-dev/reviewers-and-approvers |
Makefile
Outdated
| go install google.golang.org/protobuf/cmd/protoc-gen-go@v1.31.0 | ||
| go install google.golang.org/grpc/cmd/protoc-gen-go-grpc@v1.3.0 | ||
|
|
||
| install-go-ci-dependencies: |
There was a problem hiding this comment.
this is all commented out, do we need it?
There was a problem hiding this comment.
commented out as I'm not sure if this setting is good/ready for CI test.
Makefile
Outdated
| # go install golang.org/x/tools/cmd/goimports | ||
| # python -m pip install "pybindgen==0.22.1" "grpcio-tools>=1.56.2,<2" "mypy-protobuf>=3.1" | ||
|
|
||
| build-go: compile-protos-go |
There was a problem hiding this comment.
the inline seems to be at odds with the rest of the Makefile format, could you adjust it?
Makefile
Outdated
| install-feast-ci-locally: | ||
| pip install -e ".[ci]" | ||
|
|
||
| test-go: compile-protos-go compile-protos-python install-feast-ci-locally |
Makefile
Outdated
| format-go: | ||
| gofmt -s -w go/ | ||
|
|
||
| lint-go: compile-protos-go |
| } | ||
|
|
||
| if fs.transformationCallback != nil { | ||
| if fs.transformationCallback != nil || fs.transformationService != nil { |
There was a problem hiding this comment.
When we go to unify the transformations we'll have to update here too #4584
| return featureViewIndices, indicesFeatureView, index | ||
| } | ||
|
|
||
| func (r *RedisOnlineStore) buildHsetKeys(featureViewNames []string, featureNames []string, indicesFeatureView map[int]string, index int) ([]string, []string) { |
There was a problem hiding this comment.
nit:
| func (r *RedisOnlineStore) buildHsetKeys(featureViewNames []string, featureNames []string, indicesFeatureView map[int]string, index int) ([]string, []string) { | |
| func (r *RedisOnlineStore) buildRedisHashSetKeys(featureViewNames []string, featureNames []string, indicesFeatureView map[int]string, index int) ([]string, []string) { |
| return nil, fmt.Errorf("cannot find convert sqlite path to string %s", db_path) | ||
| } else { | ||
| store.path = fmt.Sprintf("%s/%s", repoConfig.RepoPath, dbPathStr) | ||
|
|
There was a problem hiding this comment.
was this just a lint format change?
| r := &Registry{ | ||
| cachedRegistryProtoTtl: time.Duration(registryConfig.CacheTtlSeconds), | ||
| project: project, | ||
| cachedRegistryProtoTtl: time.Duration(registryConfig.CacheTtlSeconds) * time.Second, |
There was a problem hiding this comment.
This seems like a pretty large change in time proportional to the number of seconds, no? Was this on purpose?
There was a problem hiding this comment.
This one may need @EXPEbdodla 's help to explain.
I assume the CacheTtlSeconds means TTL in the unit of second. If that is our original design, we should be good.
| } | ||
| } | ||
|
|
||
| func CallTransformations( |
Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
…#4665) * feat: Update the go feature server from Expedia code repo. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Add go package definition to RegistryServer and Grpcserver. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Fix the make build-go Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Fix makefile to make test-go work. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Removed and commented out DataDog related observability instrumentation code. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * doc: Update the README to mention the contirbution from Expedia Group. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Remove the HTTP based Registry. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Use a general string to represent the transformation service endpoint. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Set the transformation service endpoint defintion to feature_store.yaml file instead of OS env. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Fix few format issues. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> --------- Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
# [0.42.0](v0.41.0...v0.42.0) (2024-12-05) ### Bug Fixes * Add adapters for sqlite datetime conversion ([#4797](#4797)) ([e198b17](e198b17)) * Added grpcio extras to default feature-server image ([#4737](#4737)) ([e9cd373](e9cd373)) * Changing node version in release ([7089918](7089918)) * Feast create empty online table when FeatureView attribute online=False ([#4666](#4666)) ([237c453](237c453)) * Fix db store types in Operator CRD ([#4798](#4798)) ([f09339e](f09339e)) * Fix the config issue for postgres ([#4776](#4776)) ([a36f7e5](a36f7e5)) * Fixed example materialize-incremental and improved explanation ([#4734](#4734)) ([ca8a7ab](ca8a7ab)) * Fixed SparkSource docstrings so it wouldn't used inhereted class docstrings ([#4722](#4722)) ([32e6aa1](32e6aa1)) * Fixing PGVector integration tests ([#4778](#4778)) ([88a0320](88a0320)) * Incorrect type passed to assert_permissions in materialize endpoints ([#4727](#4727)) ([b72c2da](b72c2da)) * Issue of DataSource subclasses using parent abstract class docstrings ([#4730](#4730)) ([b24acd5](b24acd5)) * Operator envVar positioning & tls.SecretRef.Name ([#4806](#4806)) ([1115d96](1115d96)) * Populates project created_time correctly according to created ti… ([#4686](#4686)) ([a61b93c](a61b93c)) * Reduce feast-server container image size & fix dev image build ([#4781](#4781)) ([ccc9aea](ccc9aea)) * Removed version func from feature_store.py ([#4748](#4748)) ([f902bb9](f902bb9)) * Support registry instantiation for read-only users ([#4719](#4719)) ([ca3d3c8](ca3d3c8)) * Syntax Error in BigQuery While Retrieving Columns that Start wit… ([#4713](#4713)) ([60fbc62](60fbc62)) * Update release version in a pertinent Operator file ([#4708](#4708)) ([764a8a6](764a8a6)) ### Features * Add api contract to fastapi docs ([#4721](#4721)) ([1a165c7](1a165c7)) * Add Couchbase as an online store ([#4637](#4637)) ([824859b](824859b)) * Add Operator support for spec.feastProject & status.applied fields ([#4656](#4656)) ([430ac53](430ac53)) * Add services functionality to Operator ([#4723](#4723)) ([d1d80c0](d1d80c0)) * Add TLS support to the Operator ([#4796](#4796)) ([a617a6c](a617a6c)) * Added feast Go operator db stores support ([#4771](#4771)) ([3302363](3302363)) * Added support for setting env vars in feast services in feast controller ([#4739](#4739)) ([84b24b5](84b24b5)) * Adding docs outlining native Python transformations on singletons ([#4741](#4741)) ([0150278](0150278)) * Adding first feast operator e2e test. ([#4791](#4791)) ([8339f8d](8339f8d)) * Adding github action to run the operator end-to-end tests. ([#4762](#4762)) ([d8ccb00](d8ccb00)) * Adding ssl support for registry server. ([#4718](#4718)) ([ccf7a55](ccf7a55)) * Adding SSL support for the React UI server and feast UI command. ([#4736](#4736)) ([4a89252](4a89252)) * Adding support for native Python transformations on a single dictionary ([#4724](#4724)) ([9bbc1c6](9bbc1c6)) * Adding TLS support for offline server. ([#4744](#4744)) ([5d8d03f](5d8d03f)) * Building the feast image ([#4775](#4775)) ([6635dde](6635dde)) * File persistence definition and implementation ([#4742](#4742)) ([3bad4a1](3bad4a1)) * Object store persistence in operator ([#4758](#4758)) ([0ae86da](0ae86da)) * OIDC authorization in Feast Operator ([#4801](#4801)) ([eb111d6](eb111d6)) * Operator will create k8s serviceaccount for each feast service ([#4767](#4767)) ([cde5760](cde5760)) * Printing more verbose logs when we start the offline server ([#4660](#4660)) ([9d8d3d8](9d8d3d8)) * PVC configuration and impl ([#4750](#4750)) ([785a190](785a190)) * Qdrant vectorstore support ([#4689](#4689)) ([86573d2](86573d2)) * RBAC Authorization in Feast Operator ([#4786](#4786)) ([0ef5acc](0ef5acc)) * Support for nested timestamp fields in Spark Offline store ([#4740](#4740)) ([d4d94f8](d4d94f8)) * Update the go feature server from Expedia code repo. ([#4665](#4665)) ([6406625](6406625)) * Updated feast Go operator db stores ([#4809](#4809)) ([2c5a6b5](2c5a6b5)) * Updated sample secret following review ([#4811](#4811)) ([dc9f825](dc9f825))
…#4665) * feat: Update the go feature server from Expedia code repo. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Add go package definition to RegistryServer and Grpcserver. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Fix the make build-go Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Fix makefile to make test-go work. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Removed and commented out DataDog related observability instrumentation code. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * doc: Update the README to mention the contirbution from Expedia Group. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Remove the HTTP based Registry. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Use a general string to represent the transformation service endpoint. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Set the transformation service endpoint defintion to feature_store.yaml file instead of OS env. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> * fix: Fix few format issues. Signed-off-by: Shuchu Han <shuchu.han@gmail.com> --------- Signed-off-by: Shuchu Han <shuchu.han@gmail.com>
* Add adapters for sqlite datetime conversion ([feast-dev#4797](feast-dev#4797)) ([e198b17](feast-dev@e198b17)) * Added grpcio extras to default feature-server image ([feast-dev#4737](feast-dev#4737)) ([e9cd373](feast-dev@e9cd373)) * Changing node version in release ([7089918](feast-dev@7089918)) * Feast create empty online table when FeatureView attribute online=False ([feast-dev#4666](feast-dev#4666)) ([237c453](feast-dev@237c453)) * Fix db store types in Operator CRD ([feast-dev#4798](feast-dev#4798)) ([f09339e](feast-dev@f09339e)) * Fix the config issue for postgres ([feast-dev#4776](feast-dev#4776)) ([a36f7e5](feast-dev@a36f7e5)) * Fixed example materialize-incremental and improved explanation ([feast-dev#4734](feast-dev#4734)) ([ca8a7ab](feast-dev@ca8a7ab)) * Fixed SparkSource docstrings so it wouldn't used inhereted class docstrings ([feast-dev#4722](feast-dev#4722)) ([32e6aa1](feast-dev@32e6aa1)) * Fixing PGVector integration tests ([feast-dev#4778](feast-dev#4778)) ([88a0320](feast-dev@88a0320)) * Incorrect type passed to assert_permissions in materialize endpoints ([feast-dev#4727](feast-dev#4727)) ([b72c2da](feast-dev@b72c2da)) * Issue of DataSource subclasses using parent abstract class docstrings ([feast-dev#4730](feast-dev#4730)) ([b24acd5](feast-dev@b24acd5)) * Operator envVar positioning & tls.SecretRef.Name ([feast-dev#4806](feast-dev#4806)) ([1115d96](feast-dev@1115d96)) * Populates project created_time correctly according to created ti… ([feast-dev#4686](feast-dev#4686)) ([a61b93c](feast-dev@a61b93c)) * Reduce feast-server container image size & fix dev image build ([feast-dev#4781](feast-dev#4781)) ([ccc9aea](feast-dev@ccc9aea)) * Removed version func from feature_store.py ([feast-dev#4748](feast-dev#4748)) ([f902bb9](feast-dev@f902bb9)) * Support registry instantiation for read-only users ([feast-dev#4719](feast-dev#4719)) ([ca3d3c8](feast-dev@ca3d3c8)) * Syntax Error in BigQuery While Retrieving Columns that Start wit… ([feast-dev#4713](feast-dev#4713)) ([60fbc62](feast-dev@60fbc62)) * Update release version in a pertinent Operator file ([feast-dev#4708](feast-dev#4708)) ([764a8a6](feast-dev@764a8a6)) * Add api contract to fastapi docs ([feast-dev#4721](feast-dev#4721)) ([1a165c7](feast-dev@1a165c7)) * Add Couchbase as an online store ([feast-dev#4637](feast-dev#4637)) ([824859b](feast-dev@824859b)) * Add Operator support for spec.feastProject & status.applied fields ([feast-dev#4656](feast-dev#4656)) ([430ac53](feast-dev@430ac53)) * Add services functionality to Operator ([feast-dev#4723](feast-dev#4723)) ([d1d80c0](feast-dev@d1d80c0)) * Add TLS support to the Operator ([feast-dev#4796](feast-dev#4796)) ([a617a6c](feast-dev@a617a6c)) * Added feast Go operator db stores support ([feast-dev#4771](feast-dev#4771)) ([3302363](feast-dev@3302363)) * Added support for setting env vars in feast services in feast controller ([feast-dev#4739](feast-dev#4739)) ([84b24b5](feast-dev@84b24b5)) * Adding docs outlining native Python transformations on singletons ([feast-dev#4741](feast-dev#4741)) ([0150278](feast-dev@0150278)) * Adding first feast operator e2e test. ([feast-dev#4791](feast-dev#4791)) ([8339f8d](feast-dev@8339f8d)) * Adding github action to run the operator end-to-end tests. ([feast-dev#4762](feast-dev#4762)) ([d8ccb00](feast-dev@d8ccb00)) * Adding ssl support for registry server. ([feast-dev#4718](feast-dev#4718)) ([ccf7a55](feast-dev@ccf7a55)) * Adding SSL support for the React UI server and feast UI command. ([feast-dev#4736](feast-dev#4736)) ([4a89252](feast-dev@4a89252)) * Adding support for native Python transformations on a single dictionary ([feast-dev#4724](feast-dev#4724)) ([9bbc1c6](feast-dev@9bbc1c6)) * Adding TLS support for offline server. ([feast-dev#4744](feast-dev#4744)) ([5d8d03f](feast-dev@5d8d03f)) * Building the feast image ([feast-dev#4775](feast-dev#4775)) ([6635dde](feast-dev@6635dde)) * File persistence definition and implementation ([feast-dev#4742](feast-dev#4742)) ([3bad4a1](feast-dev@3bad4a1)) * Object store persistence in operator ([feast-dev#4758](feast-dev#4758)) ([0ae86da](feast-dev@0ae86da)) * OIDC authorization in Feast Operator ([feast-dev#4801](feast-dev#4801)) ([eb111d6](feast-dev@eb111d6)) * Operator will create k8s serviceaccount for each feast service ([feast-dev#4767](feast-dev#4767)) ([cde5760](feast-dev@cde5760)) * Printing more verbose logs when we start the offline server ([feast-dev#4660](feast-dev#4660)) ([9d8d3d8](feast-dev@9d8d3d8)) * PVC configuration and impl ([feast-dev#4750](feast-dev#4750)) ([785a190](feast-dev@785a190)) * Qdrant vectorstore support ([feast-dev#4689](feast-dev#4689)) ([86573d2](feast-dev@86573d2)) * RBAC Authorization in Feast Operator ([feast-dev#4786](feast-dev#4786)) ([0ef5acc](feast-dev@0ef5acc)) * Support for nested timestamp fields in Spark Offline store ([feast-dev#4740](feast-dev#4740)) ([d4d94f8](feast-dev@d4d94f8)) * Update the go feature server from Expedia code repo. ([feast-dev#4665](feast-dev#4665)) ([6406625](feast-dev@6406625)) * Updated feast Go operator db stores ([feast-dev#4809](feast-dev#4809)) ([2c5a6b5](feast-dev@2c5a6b5)) * Updated sample secret following review ([feast-dev#4811](feast-dev#4811)) ([dc9f825](feast-dev@dc9f825))
What this PR does / why we need it:
Update the improved Go feature server code from ExpediaGroup/Feast to our code repo.
Which issue(s) this PR fixes: