[Author Prev][Author Next][Thread Prev][Thread Next][Author Index][Thread Index]
Re: [tor-bugs] #20049 [Metrics/Metrics website]: Adapt legacy module to accept bridge network statuses from two authorities
#20049: Adapt legacy module to accept bridge network statuses from two authorities
-------------------------------------+-----------------------------
Reporter: karsten | Owner:
Type: defect | Status: merge_ready
Priority: Very High | Milestone:
Component: Metrics/Metrics website | Version:
Severity: Normal | Resolution:
Keywords: | Actual Points:
Parent ID: | Points:
Reviewer: | Sponsor:
-------------------------------------+-----------------------------
Changes (by iwakeh):
* status: needs_review => merge_ready
Comment:
Some findings below; this is difficult to review b/c of the ancient code
base.
I assume it does what is intended.
* build.xml still references descriptor-1.2.0
* It could be useful to consider enums for authority listings and EnumMap,
as these might be faster than hash-maps, but ok for the moment.
* In the existing code `bw.append(line + "\n");` should be replaced by the
two statements `bw.append(line); bw.newLine();`.
* I would place a comment differently and leave out the empty else. Minor
change suggestion w/o enum change:
{{{
diff --git
a/modules/legacy/src/org/torproject/ernie/cron/network/ConsensusStatsFileHandler.java
b/modules/legacy/src/org/torproject/ernie/cron/network/ConsensusStatsFileHandler.java
index 631839d..8d51d5d 100644
---
a/modules/legacy/src/org/torproject/ernie/cron/network/ConsensusStatsFileHandler.java
+++
b/modules/legacy/src/org/torproject/ernie/cron/network/ConsensusStatsFileHandler.java
@@ -139,6 +139,8 @@ public class ConsensusStatsFileHandler {
+ "! Aborting to read this file!");
break;
}
+ /* Assume that all lines without authority nickname are based
on
+ * Tonga's network status, not Bifroest's. */
String key = parts[0] + "," + (parts.length < 4 ? "Tonga" :
parts[1]);
String value = null;
if (parts.length == 2) {
@@ -147,11 +149,8 @@ public class ConsensusStatsFileHandler {
value = key + "," + parts[1] + "," + parts[2];
} else if (parts.length == 4) {
value = key + "," + parts[2] + "," + parts[3];
- } else {
- /* Impossible, we already checked the range above. */
- }
- /* Assume that all lines without authority nickname are based
on
- * Tonga's network status, not Bifroest's. */
+ } /* No more cases as we already checked the range above. */
+
this.bridgesRaw.put(key, value);
}
br.close();
@@ -308,7 +307,8 @@ public class ConsensusStatsFileHandler {
new FileWriter(this.bridgeConsensusStatsRawFile));
bw.append("datetime,authority,brunning,brunningec2\n");
for (String line : this.bridgesRaw.values()) {
- bw.append(line + "\n");
+ bw.append(line);
+ bw.newLine();
}
bw.close();
this.logger.fine("Finished writing file "
@@ -404,7 +404,7 @@ public class ConsensusStatsFileHandler {
+ "old: " + this.bridgesRaw.lastKey());
}
} catch (ParseException e) {
- /* Can't parse the timestamp? Whatever. */
+ logger.warning("Can't parse the timestamp? Reason: " + e);
}
}
logger.info(dumpStats.toString());
}}}
------
This module should be refactored very soon! But, that's a different
ticket.
Should there also be a follow-up ticket for accommodating future authority
changes/additions more elegantly?
Ready for merge as hot-fix.
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/20049#comment:2>
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