From 8b0bfae9cadde25d73d7e663e91322ae0ecbab1b Mon Sep 17 00:00:00 2001 From: William Parsley Date: Mon, 19 Feb 2024 17:08:29 -0600 Subject: [PATCH 01/10] first shot --- go/internal/feast/featurestore.go | 5 +---- go/internal/feast/registry/repoconfig.go | 2 ++ sdk/python/feast/repo_config.py | 3 +++ 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/go/internal/feast/featurestore.go b/go/internal/feast/featurestore.go index 650cebfc23c..f3752381dc9 100644 --- a/go/internal/feast/featurestore.go +++ b/go/internal/feast/featurestore.go @@ -58,10 +58,7 @@ func NewFeatureStore(config *registry.RepoConfig, callback transformation.Transf if err != nil { return nil, err } - sanitizedProjectName := strings.Replace(config.Project, "_", "-", -1) - productName := os.Getenv("PRODUCT") - endpoint := fmt.Sprintf("%s-transformations.%s.svc.cluster.local:80", sanitizedProjectName, productName) - transformationService, _ := transformation.NewGrpcTransformationService(config, endpoint) + transformationService, _ := transformation.NewGrpcTransformationService(config, config.GoTransformationsEndpoint) return &FeatureStore{ config: config, diff --git a/go/internal/feast/registry/repoconfig.go b/go/internal/feast/registry/repoconfig.go index c878928e6a4..c000ff9b6d4 100644 --- a/go/internal/feast/registry/repoconfig.go +++ b/go/internal/feast/registry/repoconfig.go @@ -32,6 +32,8 @@ type RepoConfig struct { RepoPath string `json:"repo_path"` // EntityKeySerializationVersion EntityKeySerializationVersion int64 `json:"entity_key_serialization_version"` + // Transformation server base endpoint + GoTransformationsEndpoint string `json:"go_transformations_endpoint"` } type RegistryConfig struct { diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index bdf063f6e47..aacf8da45ab 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -174,6 +174,9 @@ class RepoConfig(FeastBaseModel): go_feature_serving: Optional[bool] = False """ If True, use the Go feature server instead of the Python feature server. """ + go_transformations_endpoint: Optional[StrictStr] + """ Specify the URL for the Go feature server to hit the transformations server. """ + go_feature_retrieval: Optional[bool] = False """ If True, use the embedded Go code to retrieve features instead of the Python SDK. """ From 313cb92cb1c6f073cc5d4ce2ae2347c82fc1488c Mon Sep 17 00:00:00 2001 From: William Parsley Date: Mon, 19 Feb 2024 17:14:36 -0600 Subject: [PATCH 02/10] compilation errors --- go/internal/feast/featurestore.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/go/internal/feast/featurestore.go b/go/internal/feast/featurestore.go index f3752381dc9..4df1cb15e90 100644 --- a/go/internal/feast/featurestore.go +++ b/go/internal/feast/featurestore.go @@ -3,9 +3,6 @@ package feast import ( "context" "errors" - "fmt" - "os" - "strings" "github.com/apache/arrow/go/v8/arrow/memory" From 084ee0b2c9780e9f33f04f19b9e05b228ded521d Mon Sep 17 00:00:00 2001 From: William Parsley Date: Mon, 19 Feb 2024 18:19:09 -0600 Subject: [PATCH 03/10] add check for no endpoint --- go/internal/feast/featurestore.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/go/internal/feast/featurestore.go b/go/internal/feast/featurestore.go index 4df1cb15e90..08c1d0c4f9e 100644 --- a/go/internal/feast/featurestore.go +++ b/go/internal/feast/featurestore.go @@ -55,7 +55,11 @@ func NewFeatureStore(config *registry.RepoConfig, callback transformation.Transf if err != nil { return nil, err } - transformationService, _ := transformation.NewGrpcTransformationService(config, config.GoTransformationsEndpoint) + + var transformationService *transformation.GrpcTransformationService = nil + if config.GoTransformationsEndpoint != "" { + transformationService, _ = transformation.NewGrpcTransformationService(config, config.GoTransformationsEndpoint) + } return &FeatureStore{ config: config, From 3391553a2364ae1a48e4f8dae69c5a887ce82461 Mon Sep 17 00:00:00 2001 From: William Parsley Date: Wed, 21 Feb 2024 14:59:18 -0600 Subject: [PATCH 04/10] null check --- go/internal/feast/featurestore.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go/internal/feast/featurestore.go b/go/internal/feast/featurestore.go index 08c1d0c4f9e..7e38ed0ce51 100644 --- a/go/internal/feast/featurestore.go +++ b/go/internal/feast/featurestore.go @@ -57,7 +57,7 @@ func NewFeatureStore(config *registry.RepoConfig, callback transformation.Transf } var transformationService *transformation.GrpcTransformationService = nil - if config.GoTransformationsEndpoint != "" { + if config.GoTransformationsEndpoint != "" && config.GoTransformationsEndpoint != "null" { transformationService, _ = transformation.NewGrpcTransformationService(config, config.GoTransformationsEndpoint) } From f2d76541440431b49fe04c048dc09ab150106719 Mon Sep 17 00:00:00 2001 From: William Parsley Date: Thu, 22 Feb 2024 13:36:16 -0600 Subject: [PATCH 05/10] test case --- go/internal/feast/registry/repoconfig_test.go | 25 +++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/go/internal/feast/registry/repoconfig_test.go b/go/internal/feast/registry/repoconfig_test.go index 848977886c9..c88c6783236 100644 --- a/go/internal/feast/registry/repoconfig_test.go +++ b/go/internal/feast/registry/repoconfig_test.go @@ -96,3 +96,28 @@ online_store: assert.Equal(t, dir, config.RepoPath) assert.Equal(t, "data/registry.db", config.GetRegistryConfig().Path) } + +func TestGoTransformationsEndpoint(t *testing.T) { + dir, err := os.MkdirTemp("", "feature_repo_*") + assert.Nil(t, err) + defer func() { + assert.Nil(t, os.RemoveAll(dir)) + }() + filePath := filepath.Join(dir, "feature_store.yaml") + data := []byte(` +registry: + path: data/registry.db +project: feature_repo +provider: local +online_store: + type: redis + connection_string: "localhost:6379" +go_transformations_endpoint: https://go.dev:9999 +`) + err = os.WriteFile(filePath, data, 0666) + assert.Nil(t, err) + config, err := NewRepoConfigFromFile(dir) + assert.Nil(t, err) + assert.Equal(t, dir, config.RepoPath) + assert.Equal(t, "https://go.dev:9999", config.GoTransformationsEndpoint) +} From 348180dedbb0f21cdd0d8c5eedd13d2aa5bea284 Mon Sep 17 00:00:00 2001 From: William Parsley Date: Thu, 22 Feb 2024 13:59:51 -0600 Subject: [PATCH 06/10] remove possibly superfluous python change --- sdk/python/feast/repo_config.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index aacf8da45ab..bdf063f6e47 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -174,9 +174,6 @@ class RepoConfig(FeastBaseModel): go_feature_serving: Optional[bool] = False """ If True, use the Go feature server instead of the Python feature server. """ - go_transformations_endpoint: Optional[StrictStr] - """ Specify the URL for the Go feature server to hit the transformations server. """ - go_feature_retrieval: Optional[bool] = False """ If True, use the embedded Go code to retrieve features instead of the Python SDK. """ From 80857eee97676c41e28dfe081361a70c1420d7ca Mon Sep 17 00:00:00 2001 From: William Parsley Date: Mon, 26 Feb 2024 16:23:17 -0600 Subject: [PATCH 07/10] cleanup - feedback - indentation --- go/internal/feast/featurestore.go | 12 ++++++++---- go/internal/feast/registry/repoconfig.go | 2 ++ go/internal/feast/registry/repoconfig_test.go | 2 ++ sdk/python/feast/repo_config.py | 8 ++++++++ 4 files changed, 20 insertions(+), 4 deletions(-) diff --git a/go/internal/feast/featurestore.go b/go/internal/feast/featurestore.go index 7e38ed0ce51..b5ca8ecbfac 100644 --- a/go/internal/feast/featurestore.go +++ b/go/internal/feast/featurestore.go @@ -56,10 +56,14 @@ func NewFeatureStore(config *registry.RepoConfig, callback transformation.Transf return nil, err } - var transformationService *transformation.GrpcTransformationService = nil - if config.GoTransformationsEndpoint != "" && config.GoTransformationsEndpoint != "null" { - transformationService, _ = transformation.NewGrpcTransformationService(config, config.GoTransformationsEndpoint) - } + if config.GoTransformationsEndpoint == "" && config.GoTransformationsServer { + return nil, errors.New("transformations server endpoint is missing") + } + + var transformationService *transformation.GrpcTransformationService = nil + if config.GoTransformationsEndpoint != "" { + transformationService, _ = transformation.NewGrpcTransformationService(config, config.GoTransformationsEndpoint) + } return &FeatureStore{ config: config, diff --git a/go/internal/feast/registry/repoconfig.go b/go/internal/feast/registry/repoconfig.go index c000ff9b6d4..264f7ac11c0 100644 --- a/go/internal/feast/registry/repoconfig.go +++ b/go/internal/feast/registry/repoconfig.go @@ -32,6 +32,8 @@ type RepoConfig struct { RepoPath string `json:"repo_path"` // EntityKeySerializationVersion EntityKeySerializationVersion int64 `json:"entity_key_serialization_version"` + // If false, use gopy bindings to calculate ODFV transformations + GoTransformationsServer bool `json:"go_transformations_server"` // Transformation server base endpoint GoTransformationsEndpoint string `json:"go_transformations_endpoint"` } diff --git a/go/internal/feast/registry/repoconfig_test.go b/go/internal/feast/registry/repoconfig_test.go index c88c6783236..edfeb675991 100644 --- a/go/internal/feast/registry/repoconfig_test.go +++ b/go/internal/feast/registry/repoconfig_test.go @@ -113,6 +113,7 @@ online_store: type: redis connection_string: "localhost:6379" go_transformations_endpoint: https://go.dev:9999 +go_transformations_server: True `) err = os.WriteFile(filePath, data, 0666) assert.Nil(t, err) @@ -120,4 +121,5 @@ go_transformations_endpoint: https://go.dev:9999 assert.Nil(t, err) assert.Equal(t, dir, config.RepoPath) assert.Equal(t, "https://go.dev:9999", config.GoTransformationsEndpoint) + assert.Equal(t, true, config.GoTransformationsServer) } diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index bdf063f6e47..75780cd01d1 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -177,6 +177,14 @@ class RepoConfig(FeastBaseModel): go_feature_retrieval: Optional[bool] = False """ If True, use the embedded Go code to retrieve features instead of the Python SDK. """ + go_transformations_server: Optional[bool] = True + """ If True, use the transformations server to perform ODVF transformations in Go feature server. """ + + go_transformations_endpoint: Optional[StrictStr] = '' + """ Specify the endpoint for Go feature server to find the transformations server. + NOTE: Unless go_transformations_server is False, the Go feature server will throw errors if this is + blank or null. """ + entity_key_serialization_version: StrictInt = 1 """ Entity key serialization version: This version is used to control what serialization scheme is used when writing data to the online store. From e077c609c19afdeecc75ac7128e9015eca66aec4 Mon Sep 17 00:00:00 2001 From: William Parsley Date: Thu, 29 Feb 2024 14:25:15 -0600 Subject: [PATCH 08/10] fix python linting --- sdk/python/feast/repo_config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sdk/python/feast/repo_config.py b/sdk/python/feast/repo_config.py index 75780cd01d1..f1fd97e580f 100644 --- a/sdk/python/feast/repo_config.py +++ b/sdk/python/feast/repo_config.py @@ -180,7 +180,7 @@ class RepoConfig(FeastBaseModel): go_transformations_server: Optional[bool] = True """ If True, use the transformations server to perform ODVF transformations in Go feature server. """ - go_transformations_endpoint: Optional[StrictStr] = '' + go_transformations_endpoint: Optional[StrictStr] = "" """ Specify the endpoint for Go feature server to find the transformations server. NOTE: Unless go_transformations_server is False, the Go feature server will throw errors if this is blank or null. """ From aa1e2a1b3d7840dc39f165e91bcf613687e37a49 Mon Sep 17 00:00:00 2001 From: William Parsley Date: Thu, 29 Feb 2024 14:33:21 -0600 Subject: [PATCH 09/10] fix indentation --- go/internal/feast/registry/repoconfig.go | 8 ++++---- go/internal/feast/registry/repoconfig_test.go | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go/internal/feast/registry/repoconfig.go b/go/internal/feast/registry/repoconfig.go index 264f7ac11c0..8caa3f262bd 100644 --- a/go/internal/feast/registry/repoconfig.go +++ b/go/internal/feast/registry/repoconfig.go @@ -32,10 +32,10 @@ type RepoConfig struct { RepoPath string `json:"repo_path"` // EntityKeySerializationVersion EntityKeySerializationVersion int64 `json:"entity_key_serialization_version"` - // If false, use gopy bindings to calculate ODFV transformations - GoTransformationsServer bool `json:"go_transformations_server"` - // Transformation server base endpoint - GoTransformationsEndpoint string `json:"go_transformations_endpoint"` + // If false, use gopy bindings to calculate ODFV transformations + GoTransformationsServer bool `json:"go_transformations_server"` + // Transformation server base endpoint + GoTransformationsEndpoint string `json:"go_transformations_endpoint"` } type RegistryConfig struct { diff --git a/go/internal/feast/registry/repoconfig_test.go b/go/internal/feast/registry/repoconfig_test.go index edfeb675991..11b7863ad05 100644 --- a/go/internal/feast/registry/repoconfig_test.go +++ b/go/internal/feast/registry/repoconfig_test.go @@ -120,6 +120,6 @@ go_transformations_server: True config, err := NewRepoConfigFromFile(dir) assert.Nil(t, err) assert.Equal(t, dir, config.RepoPath) - assert.Equal(t, "https://go.dev:9999", config.GoTransformationsEndpoint) - assert.Equal(t, true, config.GoTransformationsServer) + assert.Equal(t, "https://go.dev:9999", config.GoTransformationsEndpoint) + assert.Equal(t, true, config.GoTransformationsServer) } From 6bafad769538ddbb1aa997a302e4207d7f65afb3 Mon Sep 17 00:00:00 2001 From: William Parsley Date: Mon, 4 Mar 2024 10:48:30 -0600 Subject: [PATCH 10/10] feedback --- go/internal/feast/featurestore.go | 9 ++++----- go/internal/feast/registry/repoconfig.go | 5 +++-- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/go/internal/feast/featurestore.go b/go/internal/feast/featurestore.go index b5ca8ecbfac..e3678cf4097 100644 --- a/go/internal/feast/featurestore.go +++ b/go/internal/feast/featurestore.go @@ -56,12 +56,11 @@ func NewFeatureStore(config *registry.RepoConfig, callback transformation.Transf return nil, err } - if config.GoTransformationsEndpoint == "" && config.GoTransformationsServer { - return nil, errors.New("transformations server endpoint is missing") - } - var transformationService *transformation.GrpcTransformationService = nil - if config.GoTransformationsEndpoint != "" { + if config.GoTransformationsServer { + if config.GoTransformationsEndpoint == "" { + return nil, errors.New("Transformations server endpoint is missing. Update featue_store.yaml with go_transformations_endpint configuration") + } transformationService, _ = transformation.NewGrpcTransformationService(config, config.GoTransformationsEndpoint) } diff --git a/go/internal/feast/registry/repoconfig.go b/go/internal/feast/registry/repoconfig.go index 8caa3f262bd..3b4e8d21ebd 100644 --- a/go/internal/feast/registry/repoconfig.go +++ b/go/internal/feast/registry/repoconfig.go @@ -32,9 +32,10 @@ type RepoConfig struct { RepoPath string `json:"repo_path"` // EntityKeySerializationVersion EntityKeySerializationVersion int64 `json:"entity_key_serialization_version"` - // If false, use gopy bindings to calculate ODFV transformations + // If false, use gopy bindings to calculate ODFV transformations. + // "True" value required for Go feature server to serve ODFVs with stability and at scale. GoTransformationsServer bool `json:"go_transformations_server"` - // Transformation server base endpoint + // Transformation server base endpoint. GoTransformationsEndpoint string `json:"go_transformations_endpoint"` }