Hi, Kathey, this is great feedback, thanks.
I get your point about the "scratch your own itch" model and basically keeping a patch focused on the initial description.
To me, my itch around this item is debugging, I have had to do a lot of this in the field, and missing something like a unique identifer for a connection can be pretty serious in terms of time spent trying to track down a bug, especially if the bug only rears it head in specific production situations, or it has only happened once but the impact was severe.
So I have been motivated to do this "right." I suspect using the default toString would have been satisfactory, but the effort required to do it "right" has been minimal once we figured out what "right" was. Most of this has been a lot of email discussion. The actual code changes have been quite small and pretty manageable. The issues around code sharing, VTIs, etc., that require larger code changes I do NOT want to do as part of this patch, and will be logging them as separate JIRA items.
I think there's value in a simple fix bringing up a larger issue for discussion. I agree that you don't want to try and *fix* these larger issues as part of the initial bug, rather they should be added as new feature requests or bugs. But I think they are healthy discussions to have and can improve the overall design and quality of the product. One of the values of open source as I see it is that we can all see and discuss problems in the architecture or design and if someone wants to put in the effort to fix it, they can.
In terms of fixing what was intended to be fixed by the initial bug report, perhaps we should try to be more clear in our definition of the situation where the bug occurred and what the expected behavior is, as you are suggesting in this email. This can be done with a reproduction, a background description in a comment, or a combination. I can see a lot of potential for someone without the background of the person who who logged the bug to fix the wrong thing...
Thanks,
David
Kathey Marsden wrote:
David Van Couvering wrote:
Daniel John Debrunner wrote:
David Van Couvering wrote:
I haven't heard any responses on this, so I am going to make the following changes/actions. Speak now or... speak later, I guess :)
- Use UUID to generate a unique id. On the engine side I will use the UUID service; on the client side I will use BasicUUID hardcoded.
I want to interject a little history here because I am concerned that I
may have misled you when I rephrased the issue description. The
original issue summary was something like "connection toString should
provide more information". The request came from a user that was using the toString() method in development to help figure out which connection
was which.
When he did this he found that we have overloaded toString() with a
method that just printed the class name. Here is the toString() from
EmbedConnection.java
public String toString() { return "EmbedConnection"; }
So the user filed this request for more information on Connection.toString() perhaps with an implied, "Why would you override the toString() method to provide *less* information?" I transferred the case to Jira, thinking probably someone else would find our toString() implementation irritating and would want to fix it.
Looking at the case originally, I kind of figured that probably we had
overridden the toString() method to address some testing concern that
stemmed out of our diff based tests. I figured if I were to look at
this case I would probably just write a test to perform toString() on all different flavors of connections, embedded, client, connections
obtained from pooled and xa connections, jdbc:default connections in
stored procedures, etc and figure out what we were doing for toString()
in all the cases. Then in the end I would have hoped to just rip out
that toString() override and resolve whatever testing concern caused it
to be there in the first place. That way we would just be using the
default toString() implementation which is pretty well defined in any
scenario and certainly would provide *more* information without a lot of
work.
When Dan suggested that I clarify the bug, I changed the description to
"connection toString should uniquely identify the connection" but to
tell you the truth now I think the original summary was probably just
fine, because it communicated the fact that our toString()
implementation should provide *more* information than the default
toString() implementation and not *less*. I couldn't say much about
where I thought the testing above might lead and whether in fact in the
end we could just use the default toString() method, because I'd really
have to do the analysis to find out. so I thought that description
would be more generic but instead, my description if taken as a
mandate, forces us to
- Define and lock in to some definition of a unique connection.
- Think about how to generate unique identifiers, in the context of
class loaders, client server environments,
clustered evironments etc.
- Think about code sharing between client and server.
- Think about the existing VTI's such as the ErrorLogReader VTI and
perhaps new VTI's
- Analyze the exisisting ID's that we have
- Consider the ease of use factor. - Consider the documentation impact of doing all this.
......I don't know what else
All of a sudden we have a major engineering effort which is maybe a great thing if all of this is important to you, but I wouldn't want DERBY-243 to be the cart that pulls this horse. If we are going down that road I think it requires a more comprehensive analysis of all the implications and dependencies than an ad hoc email conversation can provide. But I just keep wondering if such a thing is really that important to anyone right now.
So I would like to reword the description again. What do you think of a new summary for this issue?
"Connection.toString() should help identify the connection"
With description:
The Derby toString() implementation on connections overrides the default
toString() implementation and prints only the class name. This means it
is not helpful in differentiating one connection from another. It is
not clear why we do this. It may just be a testing concern, but it does
not seem to make sense that our Connection.toString() provides less
information than the default toString() implementation and not more. Even the default toString() implementation would be more helpful.
I think in general, when fixing a Jira issue, especially if it is
non-standard behavioiur it is good to consider that anyone can file any
issue for any reason. Often Jira entries can just be perceptions from
some user, or someone else's idea of what might be good. You should
pick it up and fix it if you need it or if you think it would be a
really good thing for Derby. Maybe it just will give you an idea of a
more useful thing. If your idea doesn't mesh with the original Jira
entry, just leave the original one alone and file another for the useful
thing that you are actually doing. I don't think it is good to look at
these requests as specific user requirements that need to be filled. Open source uses a "Scratch your own itch" model which is a little hard
to adjust to but I think is pretty healthy in terms of keeping the
changes useful and modulate the effort/return ratio. In this case the
new description might have been misleading but it is up to you to decide
how much you want to do with it. You might want to consider what you
think would be useful and worth the effort for Connection.toString(),
knowing that trying to go for a truly unique identifier and defining a
unique connection has all these potential implications.
I think if it were me I would .. 1) Try out different types of connection scenarios to see what we currently print. Hopefully it is just the default toString(). 2) If everywhere we are just printing the default toString() or our silly override, just get rid of our silly override. 3) Address the resulting test diffs with a <test>_sed.properties file. 4) Go on to the next thing
But as my mother in law used to say, "Little things for little minds". This is just a description of what I would do, not what you should do. I just want to make sure you aren't going to this huge unique connection, unique identifier journey on my behalf.
Kathey
