-
Notifications
You must be signed in to change notification settings - Fork 70
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
Add warning for duplicate instance name #484
Add warning for duplicate instance name #484
Conversation
a8a11de
to
99f96b2
Compare
@@ -828,6 +830,8 @@ public void init(final boolean forceNewConnection) { | |||
this.brokenInstanceMap.clear(); | |||
|
|||
final List<Instance> newInstances = new ArrayList<>(); | |||
// used to track instance names already seen | |||
Set<String> instanceNames = new HashSet<String>(); |
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.
Set<String> instanceNames = new HashSet<String>(); | |
Set<String> instanceNamesSeen = new HashSet<String>(); |
This is a good example of a comment that would be better expressed in the naming of the variable IMO
final String instanceName = (String) configInstance.get("name"); | ||
if (instanceName != null) { | ||
if (instanceNames.contains(instanceName)) { | ||
log.warn("Instance with name: {} already exists", instanceName); |
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.
log.warn("Instance with name: {} already exists", instanceName); | |
log.warn("Found multiple instances with name: '{}'. Instance names should be unique, update the 'name' field on your instances to be unique.", instanceName); |
My thought here is to have some call-to-action so that someone looking at this warning log will know what needs to happen to address this error.
(and make it the same for the json configs below)
96eba4f
to
fbf9c49
Compare
fbf9c49
to
5227dba
Compare
We currently attempt to instantiate all instances even if an instance with the same name has already been seen.
This pr adds a warn log