Skip to content

Commit

Permalink
fix: fix featctl get online feature
Browse files Browse the repository at this point in the history
  • Loading branch information
wfxr committed Nov 12, 2021
1 parent cef9a9a commit 5699d56
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 28 deletions.
5 changes: 1 addition & 4 deletions featctl/cmd/get_online_feature.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import (

type getOnlineFeatureOption struct {
types.GetOnlineFeatureValuesOpt
featureNames []string
}

var getOnlineFeatureOpt getOnlineFeatureOption
Expand All @@ -30,8 +29,6 @@ var getOnlineFeatureCmd = &cobra.Command{
oomStore := mustOpenOomStore(ctx, oomStoreCfg)
defer oomStore.Close()

// TODO: convert feature names into feature ids

featureValueMap, err := oomStore.GetOnlineFeatureValues(ctx, getOnlineFeatureOpt.GetOnlineFeatureValuesOpt)
if err != nil {
log.Fatalf("failed getting online features: %v", err)
Expand All @@ -51,7 +48,7 @@ func init() {
flags.StringVarP(&getOnlineFeatureOpt.EntityKey, "entity-key", "k", "", "entity keys")
_ = getOnlineFeatureCmd.MarkFlagRequired("entity")

flags.StringSliceVar(&getOnlineFeatureOpt.featureNames, "feature", nil, "feature names")
flags.StringSliceVar(&getOnlineFeatureOpt.FeatureNames, "feature", nil, "feature names")
_ = getOnlineFeatureCmd.MarkFlagRequired("feature")
}

Expand Down
8 changes: 4 additions & 4 deletions featctl/test/test_get_online_feature.sh
Original file line number Diff line number Diff line change
Expand Up @@ -9,17 +9,17 @@ sync $revisionID

case="query single feature"
expected='
device,model
1,xiaomi-mix3
model
xiaomi-mix3
'
actual=$(featctl get online-feature --feature model -k 1 -o csv)
assert_eq "$case" "$expected" "$actual"


case="query multiple features"
expected='
device,model,price
6,apple-iphone11,4999
model,price
apple-iphone11,4999
'
actual=$(featctl get online-feature --feature model,price -k 6 -o csv)
assert_eq "$case" "$expected" "$actual"
29 changes: 24 additions & 5 deletions internal/database/metadatav2/informer/feature.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,38 @@ func (c *FeatureCache) Enrich(groupCache *GroupCache) {
}

func (c *FeatureCache) List(opt metadatav2.ListFeatureOpt) typesv2.FeatureList {
var features typesv2.FeatureList
features := c.FeatureList

if opt.FeatureIDs == nil && opt.FeatureNames == nil && opt.EntityID == nil && opt.GroupID == nil {
features = make(typesv2.FeatureList, len(c.FeatureList))
copy(features, c.FeatureList)
return features
}

// filter ids
if opt.FeatureIDs != nil {
for _, id := range opt.FeatureIDs {
var tmp typesv2.FeatureList
for _, id := range *opt.FeatureIDs {
if f := c.Find(func(f *typesv2.Feature) bool {
return f.ID == id
}); f != nil {
features = append(features, f)
tmp = append(features, f)
}
}
features = tmp
}

// filter names
if opt.FeatureNames != nil {
var tmp typesv2.FeatureList
for _, name := range *opt.FeatureNames {
if f := features.Find(func(f *typesv2.Feature) bool {
return f.Name == name
}); f != nil {
tmp = append(tmp, f)
}
}
} else {
features = c.FeatureList
features = tmp
}

// filter entity
Expand Down
7 changes: 4 additions & 3 deletions internal/database/metadatav2/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,8 @@ type ListRevisionOpt struct {
}

type ListFeatureOpt struct {
EntityID *int16
GroupID *int16
FeatureIDs []int16
EntityID *int16
GroupID *int16
FeatureIDs *[]int16
FeatureNames *[]string
}
2 changes: 1 addition & 1 deletion pkg/oomstore/offline_feature_value.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
// GetHistoricalFeatureValues gets point-in-time feature values for each entity row;
// currently, this API only supports batch features.
func (s *OomStore) GetHistoricalFeatureValues(ctx context.Context, opt types.GetHistoricalFeatureValuesOpt) (*types.JoinResult, error) {
features := s.metadatav2.ListFeature(ctx, metadatav2.ListFeatureOpt{FeatureIDs: opt.FeatureIDs})
features := s.metadatav2.ListFeature(ctx, metadatav2.ListFeatureOpt{FeatureIDs: &opt.FeatureIDs})

features = features.Filter(func(f *typesv2.Feature) bool {
return f.Group.Category == types.BatchFeatureCategory
Expand Down
4 changes: 2 additions & 2 deletions pkg/oomstore/online_feature_value.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (
func (s *OomStore) GetOnlineFeatureValues(ctx context.Context, opt types.GetOnlineFeatureValuesOpt) (types.FeatureValueMap, error) {
m := make(map[string]interface{})

features := s.metadatav2.ListFeature(ctx, metadatav2.ListFeatureOpt{FeatureIDs: opt.FeatureIDs})
features := s.metadatav2.ListFeature(ctx, metadatav2.ListFeatureOpt{FeatureNames: &opt.FeatureNames})
features = features.Filter(func(f *typesv2.Feature) bool {
return f.Group.OnlineRevisionID != nil
})
Expand Down Expand Up @@ -49,7 +49,7 @@ func (s *OomStore) GetOnlineFeatureValues(ctx context.Context, opt types.GetOnli
}

func (s *OomStore) MultiGetOnlineFeatureValues(ctx context.Context, opt types.MultiGetOnlineFeatureValuesOpt) (types.FeatureDataSet, error) {
features := s.metadatav2.ListFeature(ctx, metadatav2.ListFeatureOpt{FeatureIDs: opt.FeatureIDs})
features := s.metadatav2.ListFeature(ctx, metadatav2.ListFeatureOpt{FeatureIDs: &opt.FeatureIDs})

features = features.Filter(func(f *typesv2.Feature) bool {
return f.OnlineRevision() != nil
Expand Down
14 changes: 7 additions & 7 deletions pkg/oomstore/online_feature_value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ func TestGetOnlineFeatureValues(t *testing.T) {
{
description: "no available features, return nil",
opt: types.GetOnlineFeatureValuesOpt{
FeatureIDs: unavailableFeatures.Ids(),
EntityKey: "1234",
FeatureNames: unavailableFeatures.Ids(),
EntityKey: "1234",
},
features: unavailableFeatures,
expectedError: nil,
Expand All @@ -49,8 +49,8 @@ func TestGetOnlineFeatureValues(t *testing.T) {
{
description: "inconsistent entity type, fail",
opt: types.GetOnlineFeatureValuesOpt{
FeatureIDs: inconsistentFeatures.Ids(),
EntityKey: "1234",
FeatureNames: inconsistentFeatures.Ids(),
EntityKey: "1234",
},
features: inconsistentFeatures,
expectedError: fmt.Errorf("inconsistent entity type: %v", map[string]string{"device": "price", "user": "age"}),
Expand All @@ -59,8 +59,8 @@ func TestGetOnlineFeatureValues(t *testing.T) {
{
description: "consistent entity type, succeed",
opt: types.GetOnlineFeatureValuesOpt{
FeatureIDs: consistentFeatures.Ids(),
EntityKey: "1234",
FeatureNames: consistentFeatures.Ids(),
EntityKey: "1234",
},
features: consistentFeatures,
entityName: &entityName,
Expand All @@ -74,7 +74,7 @@ func TestGetOnlineFeatureValues(t *testing.T) {

for _, tc := range testCases {
t.Run(tc.description, func(t *testing.T) {
metadatav2Store.EXPECT().ListFeature(gomock.Any(), metadatav2.ListFeatureOpt{FeatureIDs: tc.opt.FeatureIDs}).Return(tc.features, nil)
metadatav2Store.EXPECT().ListFeature(gomock.Any(), metadatav2.ListFeatureOpt{FeatureIDs: tc.opt.FeatureNames}).Return(tc.features, nil)
if tc.entityName != nil {
onlineStore.EXPECT().Get(gomock.Any(), gomock.Any()).Return(dbutil.RowMap{
"price": int64(100),
Expand Down
4 changes: 2 additions & 2 deletions pkg/oomstore/types/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,8 @@ type CsvDataSource struct {
}

type GetOnlineFeatureValuesOpt struct {
FeatureIDs []int16
EntityKey string
FeatureNames []string
EntityKey string
}

type MultiGetOnlineFeatureValuesOpt struct {
Expand Down

0 comments on commit 5699d56

Please sign in to comment.