[receiver/solace]: Update broker config #36387
Conversation
|
Could you find a way to avoid a breaking change here? Maybe set the broker field to any and introspect if it's a slice, in which case you'd raise a warning to move it to string. |
The problem with "any" is that a string is set as the default value. This will make the unmarshalling fail because it will use reflection to override the default broker string and will throw a decode error: 'broker' expected type 'string', got unconvertible type '[]interface {}', value: '[myHost:5671]'. But with this, I found out that the decoder accepts a string for a slice field. Maybe we could keep the broker as a slice but document it in the readme as a string and throw an error if the slice has more than 1 element in it. What do you think? |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
wildum
force-pushed
the
update-broker-config
branch
from
58cd508 to
4423fca
Compare
wildum
force-pushed
the
update-broker-config
branch
from
4423fca to
41676ec
Compare
|
@atoulme @djaglowski I also made the changes on this one, lmk if you want to move forward with it or if you don't want to change |
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Description
The broker field is a list of strings but only the first element is considered. This is misleading because users might think that the component supports connecting to several brokers but only the first broker is used in the code.
This PR is updating the doc and adding validation steps to make sure that only one broker is configured. This is a breaking change in theory because users who have several brokers configured will get an error on start-up but since it is not supported, it's unlikely that it breaks anyone.
Testing
The tests were updated.
Documentation
The examples in the doc were updated.