-
Notifications
You must be signed in to change notification settings - Fork 3
feat: pluggable materialization #211
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
Conversation
| 'https://resolver.confidence.dev/*': [ | ||
| withRouter({ | ||
| '*/v1/flags:resolve': [ | ||
| '*/v1/materialization:readMaterializedOperations|*/v1/materialization:writeMaterializedOperations': [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is reasonable to split these and allow for configuration of timeout individually.
in go and java we allow for env var configuration of this (defaulting to deadline 2 sec for read and 5 sec for write).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm happy to split them. But I'd prefer options rather than env vars? Do we want that? And/or do we also want separate defaults?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have a strong opinion on wether it is env vars or not. Totally fine with options.
I think it makes sense to allow for separate defaults, considering we are not waiting for the write we can allow for longer timeout there but might want to set the read timeout as long as milliseconds?
Introduces a MaterializationStore interface and makes the Confidence remote materialization store available.