-
Notifications
You must be signed in to change notification settings - Fork 3
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
v1 api #54
base: main
Are you sure you want to change the base?
v1 api #54
Conversation
If that's the only change then perhaps we shouldn't wait a bit longer? |
I don't mind, but I don't think there's many planned changes for this lib, so... what should we be waiting for? |
Although the v1 is easier to use because we ask for two arguments ( Since we are breaking back-guards compatibility, shouldn't we explore a deeper separation of concerns with a Factory class/function? Something like this: class TraefikRouterFactory():
@staticmethod
def get_requirer(charm: CharmBase, endpoint_name: str = "traefik-route")
events = TraefikRouteRequirerEvents()
stored = StoredState()
relation = charm.model.get_relation(endpoint_name)
return TraefikRouteRequirer(charm, events, stored, relation) This way we inject all the dependencies |
It being an |
Issue
v0's Requirer object had a really bad API:
this is bad because we can derive relation from relation_name and viceversa.
Modern libraries all take an
endpoint_name
and use it to fetch the relation(s).V1 breaks this API by exposing instead:
This is not an interface change (the interface version remains the same). But it is a breaking api change, hence the major vbump for the charm lib.