-
Notifications
You must be signed in to change notification settings - Fork 109
Reorganize Schema Registry #3876
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
Reorganize Schema Registry #3876
Conversation
* Moved SchemaRegistry in types, together with updater. * Abstract away the dependencies required by the SchemaRegistry
* Privatized the schema updater types, especially errors. * Split registry in multiple files.
* Flatten DeploymentMetadata in just Deployment * Group the arguments for add_deployment and update_deployment
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.
Thanks @slinkydeveloper for this PR. the changes in general looks good to me. I left couple of nits :)
I am also wondering if we really need the generic type parameters that is leaking everywhere, mainly <Metadata, Discovery, Telemetry>. Or can we get away with using the concrete implementations of those objects since it seems we only have mainly one implementation of each. Are they used in testing ?
|
||
#[derive(Debug, thiserror::Error, codederror::CodedError)] | ||
pub enum SchemaError { | ||
pub(in crate::schema) enum SchemaError { |
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.
TIL :)
Yes I would like to be able to test the rest api endpoints, plus I needed to abstract away metadata, discovery and telemetry to bring this inside restate_types. An example of such test is here: #3887 Originally, I wanted to just use dyn traits here, but unfortunately I cannot because they have GATs and generics :( |
This PR is a continuation of the past few PRs related to the schema registry.
The goal here it to make the module
restate_types::schema
self-contained, creating a single place where we have the schema registry:SchemaRegistry
data structureSchemaRegistry
data structure and submodulesdeployment
,service
,invocation_target
, etcSchema
data structureNow
restate_types::schema
could even be moved in its own crate (still depends on a bunch ofrestate_types
, but not too many!)This PR also simplifies some of the access logic API, such as removing the nesting
DeploymentMetadata
.