iiliev2 commented on PR #4899: URL: https://github.com/apache/activemq-artemis/pull/4899#issuecomment-2078890094
Initially I attempted what you suggest about lazy initializing the node id like that, precicely because I wanted to keep the code changes to a minimum. However, that ended up being much more complicated(rather than simplified), because of the way `ClientSessionFactoryImpl` creates a new connection object on re-connects. It is very hard to reason about both when reading the code and when needing to debug it at runtime. So instead of this, I had to fill the missing gaps to use the data that is already there anyway, just wasn't being propagated deep enough. IMO from a functional standpoint, adding the `TransportConfiguration` to the connector(and connection) is the right thing to do here anyway. I assume due to historical reasons, those classes were working with a subset of the data, and no one had a good reason to fix this until now. For example `NettyConnection#getConnectorConfig` was creating a bogus transport configuration, even though when it was created there was a configuration which was not being passed to it. `Ping` is already the only `Packet` that is being modified. Why do you want to use a raw `byte[]` rather than `UUID`? IMO that will be more confusing - it suggests that there could be other kind of data that can be contained. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
