feat: add configurable validation strategy by topic #745
Conversation
eliax1996
force-pushed
the
eliax1996/enable-users-to-switch-subject-strategy-by-topic
branch
4 times, most recently
from
a314cac
to
5541f81
Compare
eliax1996
force-pushed
the
eliax1996/enable-users-to-switch-subject-strategy-by-topic
branch
from
5541f81
to
fcd6710
Compare
eliax1996
force-pushed
the
eliax1996/enable-users-to-switch-subject-strategy-by-topic
branch
from
bce8a6e
to
50e1c25
Compare
eliax1996
force-pushed
the
eliax1996/enable-users-to-switch-subject-strategy-by-topic
branch
from
50e1c25
to
a31471c
Compare
| @@ -269,12 +282,14 @@ def validate_config(config: Config) -> None: | |||
| f"Invalid master election strategy: {master_election_strategy}, valid values are {valid_strategies}" | |||
| ) from None | |||
|
|
|||
| name_strategy = config["name_strategy"] | |||
| deafault_name_strategy = config["default_name_strategy"] | |||
There was a problem hiding this comment.
Typo
| deafault_name_strategy = config["default_name_strategy"] | |
| default_name_strategy = config["default_name_strategy"] |
There was a problem hiding this comment.
yeah you are right, I've copy-pasted the same in the following pr
| * - ``name_strategy`` | ||
| - ``topic_name`` | ||
| - Name strategy to use when storing schemas from the kafka rest proxy service | ||
| * - ``default_name_strategy`` |
There was a problem hiding this comment.
This change would require a major version upgrade
There was a problem hiding this comment.
yep, I've decided to get rid of this change. This pr needs to be rebased on top of the #754. Probably I will close this pr since the design of making stateful the request for publish using the rest endpoint isn't a great idea.
It's probably mark this pr as draft
| if topic_name not in self.topic_validation_strategies: | ||
| return None | ||
|
|
||
| return self.topic_validation_strategies[topic_name] |
There was a problem hiding this comment.
How about
self.topic_validation_strategies.get(topic_name)?
There was a problem hiding this comment.
I would prefer an exception instead of a None as return type since I'm guarding the access on the if before, am I wrong?
| @@ -39,7 +45,7 @@ | |||
| import logging | |||
| import time | |||
|
|
|||
| RECORD_KEYS = ["key", "value", "partition"] | |||
| SUBJECT_VALID_POSTFIX = [SubjectType.key, SubjectType.value] | |||
There was a problem hiding this comment.
Are we getting rid of partition here?
There was a problem hiding this comment.
yep, if you look previously there was a tricky side effect of using the zip function.
If you run zip on two lists and the first one it's shorter than the second you will get as output a list of tuple2 with the same length of the first list. And we were exactly in that specific case 😄
This pr enable the user to choose the naming strategy (and his validation) by running a
POSTtowards/topic/<topic:str>/name_strategy/<strategy:str>endpoint and get the current used one by doing aGETtowards/topic/<topic:str>/name_strategy