Fixes #24987
Comment From: wilkinsona
When merging this, we should rename SeedNode
to Seednode
to avoid a -
in the property prefix.
Comment From: snicoll
We may also want to change the condition to avoid relying on "spring.couchbase.seed-nodes[0].address"
.
@daschl what do you think? SeedNode
doesn't feel like a 100% replacement for connectionString
to me and I am wondering if we may be missing something. Ideally, I'd like whatever we introduce here to be a "super set" of what connection string can do. Thoughts?
Comment From: daschl
The SeedNode
ist just an alternate way of configuring the bootstrap host list, and is mainly only useful if you want to provide both a custom cluster manager port and a custom kv port. This mostly happens in testing if you run against custom docker setups for example.
Note that we actually internally from a connection string build up the seed nodes. Note that for the purpose of spring boot, I'm not sure if we actually need to expose it. The reason is, and maybe this is what @aaronjwhiteside is missing, you CAN actually configure both ports in the connection string like: couchbase://foo:1234=manager,foo:5678=kv
, and we'll internally coalesce the foo host to use 1234 for the cluster manager port and 5678 for the kv port.
I have to apologize though that this is not properly documented in the SDK docs, mainly because not many people needed it at all other than some internal consumers. I'll make sure this gets added.
Comment From: aaronjwhiteside
The reason is, and maybe this is what @aaronjwhiteside is missing, you CAN actually configure both ports in the connection string like: couchbase://foo:1234=manager,foo:5678=kv, and we'll internally coalesce the foo host to use 1234 for the cluster manager port and 5678 for the kv port.
Oh I am missing this!
Might want to update the documentation at https://docs.couchbase.com/java-sdk/current/howtos/managing-connections.html#alternate-addresses Where SeedNodes are the recommended way to specify alternative ports.
I also hope this isn't documented because it's not intended to be a support feature going forward?
Comment From: snicoll
Alright so I am going to close this now per @daschl comment. Thanks for the pr @aaronjwhiteside, in any case!