-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Detect exact duplicates for URLs for routes, gateways or leafnodes. #2930
Conversation
@wallyqs could you double check my warnings logic, not as familiar with that as you are. |
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.
LGTM
…, enter a warning and ignore. If misconfigured could prevent the JetStream system from electing a leader. Signed-off-by: Derek Collison <derek@nats.io>
0071a7e
to
e204a79
Compare
Thanks. Had a side effect that was making another test fail, so fixed that and want @wallyqs to eyeball the warning construction. |
Co-authored-by: Waldemar Quevedo <wally@synadia.com>
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.
LGTM
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.
LGTM.
Currently it reads like this:
examples/js-cluster.conf:9:6: invalid use of field "nats://127.0.0.1:6224": Duplicate route entry detected
examples/js-cluster.conf:11:6: invalid use of field "nats://127.0.0.1:6225": Duplicate route entry detected
The lines already point to the location so that is enough I think, but we could make the warning clearer by changing the field
from the warning error so that it reads something like this:
examples/js-cluster.conf:9:6: invalid use of field "routes": Duplicate route "nats://127.0.0.1:6224" entry detected
examples/js-cluster.conf:11:6: invalid use of field "routes": Duplicate route "nats://127.0.0.1:6225" entry detected
Have a commit here with the change showing how it could look like: c7e357e
I committed your suggestion already, maybe pull and see? lmk |
Actually will merge and we can iterate on warning as needed. |
If misconfigured could prevent the JetStream system from electing a leader. We will skip the entry and warn to the log.
Signed-off-by: Derek Collison derek@nats.io
/cc @nats-io/core