Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source data model in Feast Core should be generalized #632

Closed
woop opened this issue Apr 18, 2020 · 2 comments · Fixed by #685
Closed

Source data model in Feast Core should be generalized #632

woop opened this issue Apr 18, 2020 · 2 comments · Fixed by #685
Assignees

Comments

@woop
Copy link
Member

woop commented Apr 18, 2020

Expected Behavior

Sources are generic and extensible.

Current Behavior

Currently the sources data model in Feast Core is hardcoded specifically to KafkaSource.

Possible Solution

Make sure none of the columns in our data model is specific to a single source. The pattern we have followed thus far is to use Map<String,String> for Source, Runner, Store configurations. One way to solve this problem is to have source configuration stored as a serialized JSON map in a single column in the Source table.

@woop woop changed the title Sources should be generic Sources should be generalized Apr 18, 2020
@woop woop added this to the v0.5.0 milestone Apr 18, 2020
@ches
Copy link
Member

ches commented Apr 18, 2020

While I get the utility and convenience especially for deploying Feast with more distributed ownership in the organization, we ignore / work around a lot of the Source and Store configuration kept in the registry RDBMS, in favor of service discovery.

I also think that asking data scientists and data engineers to be concerned with operational infra configuration like Kafka broker addresses when registering feature sets is not an elegant separation of concerns.

Maybe we can take this as an opportunity to consider design alternatives as well.

@woop woop changed the title Sources should be generalized Source data model in Feast Core should be generalized Apr 19, 2020
@woop
Copy link
Member Author

woop commented Apr 19, 2020

Hi @ches, would love to continue the discussion. I've created an issue specifically for the topic over here: #633

That allows this card to stay on track separately since I don't see us making fundamental changes to sources in 0.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants