FeatureRow proto to wrap FeatureRowKey and FeatureRowMessage #13

Activity

1. woop commented on Dec 24, 2018

   

   woop

   on Dec 24, 2018

   Member

   More actions

   @baskaranz can you propose an example of the proto format that you are suggesting? What would be contained within each of the FeatureRowKey and FeatureRowMessage?
2. baskaranz commented on Dec 24, 2018

   

   baskaranz

   on Dec 24, 2018

   Last edited by woop

   ContributorAuthor

   More actions

   @woop Here's what is being proposed for the FeatureRow proto.

   The FeatureRowKey having all the elements from FeatureRowMessage except/minus the features element.

   message FeatureRowKey {
     string entityKey = 1;
     google.protobuf.Timestamp eventTimestamp = 3;
     string entityName = 4;
     Granularity.Enum granularity = 5;
   }

   message FeatureRowMessage {
     string entityKey = 1;
     **repeated Feature features = 2;**
     google.protobuf.Timestamp eventTimestamp = 3;
     string entityName = 4;
     Granularity.Enum granularity = 5;
   }

   Edit: Added formatting for code block
3. woop commented on Dec 24, 2018

   

   woop

   on Dec 24, 2018

   Last edited by woop

   Member

   More actions

   @baskaranz, I am assuming that this would replace the existing message format for Pub/Sub, meaning this would be the new standard, right?

   Does this impact Pub/Sub at all? If not, then the format looks fine to me.

   @tims, can you have a look?
4. tims commented on Dec 26, 2018

   

   tims

   on Dec 26, 2018

   Contributor

   More actions

   It looks like it wouldn't effect PubSub, since you are just renaming FeatureRow to FeatureRowMessage.

   What is the key used for in Kafka? If you leave the key as null, the message will be auto assigned to a random partition anyway no? and if you want to use the key to assign partitions, you need to ensure they are well distributed which this key does not (as entityKey is not under Feast's control and often a sequentially assigned integer).

   Feast wouldn't care much for what the key was, since it's all in the message anyway. I'm not sure there's a real need for the key to be a proto in feast, or a proto at all.

   I'm not necessarily saying we shouldn't do this, but I'd like to know more about the why.

   Also, are you working on Kafka support already? if so can you make an issue for it.
5. woop commented on Jan 3, 2019

   

   woop

   on Jan 3, 2019

   Member

   More actions

   It seems like it's just for partitioning, so leaving it as null would be a valid approach, at least for now. @ravisuhag can you see any downsides?

   Kafka issue: #22
6. tims commented on Jan 3, 2019

   

   tims

   on Jan 3, 2019

   Contributor

   More actions

   Can we close this now, I don't see a need to change FeatureRow to FeatureRowMessage. And we've already merged in a FeatureRowKey proto.
7. woop commented on Jan 3, 2019

   

   woop

   on Jan 3, 2019

   Last edited by woop

   Member

   More actions

   We could close it, but are we happy leaving the code base as is? If we don't need FeatureRowKey, then shouldn't we remove it and add support for null keys during ingestion?

   Or should that be a separate issue? @tims
8. tims commented on Jan 10, 2019

   

   tims

   on Jan 10, 2019

   Contributor

   More actions

   I'll add a note to the kafka source issue and close this one.
9. tims

   closed this as completedon Jan 10, 2019
10. Yanson

    added a commit that references this issue on Jul 29, 2020

    Created databricks-emulator (feast-dev#13)

    ...

    88e43a8
11. franciscojavierarceo

    added a commit that references this issue on Apr 15, 2025

    fix: Update java pr #13

    ...

    fda7db7
12. franciscojavierarceo

    added a commit that references this issue on Apr 29, 2025

    chore(release): release 0.49.0

    ...

    ad501cd
13. jfw-ppi

    added 2 commits that reference this issue on Jun 7, 2025

    fix: Update java pr feast-dev#13

    ...

    d06d445

    chore(release): release 0.49.0

    ...

    0394e64