Closed jendakol closed 7 years ago
The point of client-provided names was never to make it easier to load in the apps, although there's nothing wrong with that. The point is to make it visible in server logs (when a connection is accepted) and management UI (for the entire lifetime of a connection).
I'm not sure if I understand. I need it for logging :-) But anyway, the reason is IMO not relevant - is there a problem with passing it there from Lyra? AFAIK it's the same value as I would havewhen creating the connection by myself, except I don't any Lyra stops me from passing it there (well I found a way how to override it, but...).
@jendakol Happy to take a PR for this.
@jhalterman I would do the PR directly, but it's one-line change and it will take more time to me to fork the repository etc. than you to fix it, but if you insist.. ;-)
@jendakol asking the maintainer to do stuff is not how open source works, ya know.
@michaelklishin I agree in general but thought this case could be the exception. I as a maintainer wouldn't be offended if someone tell me "hey, you have a bug here, fix it with this", if it would be that simple as this is.
@jendakol You're not wrong, but at the same time for someone who works on lots of projects and doesn't have a lot of free time, getting a pull request is always a nice thing :) Ideally, a project should make it as easy as possible to fork/clone/build/test a change so that one line fixes are quick for anyone to do.
:+1:
The standard client supports passing customized connection name to
newConnection
method. Lyra should use it (https://github.com/jhalterman/lyra/blob/master/src/main/java/net/jodah/lyra/internal/ConnectionHandler.java#L243) because now the provided name (ConnectionOptions.withName
) is visible only in logs from Lyra but not available inConnection.getClientProvidedName
method (e.g. in listeners to log it). The fix should be one-liner. Thx.