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

Refactor schema interface usage to have unified naming and simpler updates #4942

Open
Tracked by #4927
reyreaud-l opened this issue May 16, 2024 · 0 comments
Open
Tracked by #4927
Assignees

Comments

@reyreaud-l
Copy link
Contributor

reyreaud-l commented May 16, 2024

Currently the implementation exposed by the cluster package for schema management is used via interfaces in multiple packages with differing naming conventions.

The goal of this task is to simplify the usage of the cluster package and have a clear consistent naming convention, so that on a callee side we can understand what the interface is used for, and most probably what implements it under the hood.

The following is a non-exhaustive list of places where we want to unify the naming/usage:

usecases/schema

  • metaWriter -> this is implemented by the cluster service where a client to the leader is available
  • metaReader -> this is implemented by the cluster schema directly that allows to read the local schema

-> Proposal here would be to unify these two interface under a SchemaReader interface. It would be implemented by a single underlying struct that give the caller the ability to read schema either from the leader or from the local schema, and update the schema via the leader

This change will rely on a refactor defined in #4944

everywhere

  • ReadOnlyClass(...) usage (mostly in adapters/repos but also in other places.
  • GetSchema(...) usage (mostly in adapters/repos but also in other places.

There are places left in the code where we still directly query the schema to get a class (from a name) or get the whole schema (to iterate over all classes). These places use a defined func getClass(string) *models.Class which we should unify to a single type definition (under cluster/types maybe ?) which the code can import. This gives us a single place to update when changing that function signature.
For GetSchema we should change it to GetAllClasses (maybe using an iterator style function?) so that we don't have a way of leaking the underlying schema implementation to the client.

``

@reyreaud-l reyreaud-l self-assigned this May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant