-
Notifications
You must be signed in to change notification settings - Fork 183
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
KV should raise an error if the key contains unacceptable characters #553
Comments
caspervonb
added
accepted
The defect or proposal as been accepted
defect
Suspected defect such as a bug or regression
and removed
proposal
Enhancement idea or proposal
labels
Jun 11, 2024
I'd consider this a defect, we should match the validation that Go does in this case (https://github.com/nats-io/nats.go/blob/main/kv.go#L558) |
Is this not the Primitive Obsession code smell? https://refactoring.guru/smells/primitive-obsession from typing import Final, Self
import re
class Key(str):
PATTERN: Final = re.compile(r"^[A-Za-z_-]+[A-Za-z0-9_-]*$") # whatever you want / need
def __new__(cls, *args, **kwargs) -> Self:
value = super().__new__(cls, *args, **kwargs)
# really any checks you require here but likely regex will suffice
if cls.PATTERN.match(value) is None:
raise ValueError(f'Key {value} contains invalid characters')
return value |
IMO these Value classes should be used elsewhere in the code-base as well. Example bucket name. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Proposed change
If the key contains unacceptable characters (e.g. whitespace), an error should be raised. Maybe
BadSubjectError
can be reused or a new error can be created (e.g.BadKeyError
).It currently gives an
UnexpectedEOF
error and causes aTimeoutError
. This is fairly confusing because it doesn't necessarily imply the key is bad and could be remedied by a quick check on the key.Use case
I had somebody else feeding me some data, which was being fed into nats kv. I noticed that occasionally, an entry would cause an eof error and a timeout error and it wasn't very clear why. I finally realized that the user was putting spaces in the keys, which was causing problems in the underlying
publish()
call.Contribution
Sure. I would assume it would be putting in a little check in
put()
The text was updated successfully, but these errors were encountered: