-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Error Store should not require a storage spec #27
Comments
Can you clarify how would you like to tell ingestion where to store the errors? |
Currently my plan is to pass it in as 2 separate options: These would be defined in the config for core feast deployment is this ok @tims? |
That looks suspiciously like a storage spec :) If we're committed to just files and logs.. we could just log to stderr by default if a file path isn't provided? I imagine we might want avro or parquet too though... hmm. Would errorsStoreOptions be comma separated kvs? eg. x=1,y=2 |
Sorry, I realised the issue was poorly worded: I meant we should not give error stores the same treatment as other storage specs currently. The fact that it (1) has to be registered (2) can be retrieved together with all other specs and (3) can be written to as a regular warehouse store by ingestion jobs is what i want to change. At the same time it would be nice to retain the flexibility to add other potential error store formats in the future, which is why I've split its definition into
I was thinking json e.g. |
I think json is ok, as long as it doesn't cause any shell escaping issues for the core api? reviewing the rest now |
separate validating functions
The error store should be defined on deployment (of core), and should not require a spec.
The text was updated successfully, but these errors were encountered: