[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]

Re: [tor-bugs] #24327 [Metrics/ExoneraTor]: Sort results under technical details by timestamp and, if necessary, by fingerprint



#24327: Sort results under technical details by timestamp and, if necessary, by
fingerprint
--------------------------------+------------------------------
 Reporter:  karsten             |          Owner:  metrics-team
     Type:  defect              |         Status:  merge_ready
 Priority:  Medium              |      Milestone:
Component:  Metrics/ExoneraTor  |        Version:
 Severity:  Normal              |     Resolution:
 Keywords:                      |  Actual Points:
Parent ID:                      |         Points:
 Reviewer:                      |        Sponsor:
--------------------------------+------------------------------

Comment (by iwakeh):

 Replying to [comment:5 karsten]:
 > Moving the sorting code to `QueryServlet` sounds good to me. Good point
 about also having to implement `equals()` and so on, that's not really
 what we want here. Changed in a subsequent commit in the same branch.

 Fine.

 >
 > Regarding your question about the reason for moving sorting from the
 database to Java, I can't give you a definite answer. We changed a few
 things when combining multiple database queries into one. The new code did
 not require ordered query results in order to produce correct output, and
 it still does not. We took out the ORDER BY statements, because that was
 easier than replacing numbers with names. We just overlooked that this
 would affect the order of entries in the technical results. Now, we could
 put the sorting back to the database, but then we'd have to find a way to
 use column names rather than numbers, and I didn't find an easy way to do
 that with all the UNIONs. I'd say it's simply easier to sort things in the
 servlet.

 Ah, ok.  Thanks for pointing this all out.

 >
 > I also agree that tests would be great. I even spent some time thinking
 about testing this in `QueryResponseTest` until realizing that it should
 rather be tested in a new `QueryServletTest`. But writing useful tests for
 `QueryServlet` is not a trivial task and shouldn't delay merging this
 bugfix.
 >

 I think this situation should be changed.  Creating new ticket #24365 for
 this task.

--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/24327#comment:7>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
_______________________________________________
tor-bugs mailing list
tor-bugs@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs