Re: [tor-bugs] #29166 [Metrics/Statistics]: Run modules from Java only

[Tor Bug Tracker & Wiki]

#29166: Run modules from Java only
+-
 Reporter:  karsten |  Owner:  karsten
 Type:  enhancement | Status:  closed
 Priority:  Medium  |  Milestone:
Component:  Metrics/Statistics  |Version:
 Severity:  Normal  | Resolution:  fixed
 Keywords:  |  Actual Points:
Parent ID:  | Points:
 Reviewer:  irl |Sponsor:
+-
Changes (by karsten):

 * status:  merge_ready => closed
 * resolution:   => fixed


Comment:

 This is deployed as of Thursday/Friday, and the weekend runs looked
 normal. Closing. Yay, this is a big step towards making metrics-lib more
 maintainable in the future!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29166 [Metrics/Statistics]: Run modules from Java only

[Tor Bug Tracker & Wiki]

#29166: Run modules from Java only
+-
 Reporter:  karsten |  Owner:  karsten
 Type:  enhancement | Status:  merge_ready
 Priority:  Medium  |  Milestone:
Component:  Metrics/Statistics  |Version:
 Severity:  Normal  | Resolution:
 Keywords:  |  Actual Points:
Parent ID:  | Points:
 Reviewer:  irl |Sponsor:
+-

Comment (by karsten):

 Cool! I ran this using the integration tests from #29425 and spotted a
 bug. Fixed that bug and pushed to master:

 {{{
 diff --git
 a/src/main/java/org/torproject/metrics/stats/connbidirect/Main.java
 b/src/main/java/org/torproject/metrics/stats/connbidirect/Main.java
 index f4743af9..c7fefee7 100644
 --- a/src/main/java/org/torproject/metrics/stats/connbidirect/Main.java
 +++ b/src/main/java/org/torproject/metrics/stats/connbidirect/Main.java
 @@ -130,7 +130,7 @@ public class Main {
static final long ONE_DAY_IN_MILLIS = 8640L;

private static final File baseDir = new File(
 -  org.torproject.metrics.stats.main.Main.modulesDir, "advbwdist");
 +  org.torproject.metrics.stats.main.Main.modulesDir, "connbidirect");

/** Executes this data-processing module. */
public static void main(String[] args) throws IOException {
 }}}

 Will deploy later today and then close.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29166 [Metrics/Statistics]: Run modules from Java only

[Tor Bug Tracker & Wiki]

#29166: Run modules from Java only
+-
 Reporter:  karsten |  Owner:  karsten
 Type:  enhancement | Status:  merge_ready
 Priority:  Medium  |  Milestone:
Component:  Metrics/Statistics  |Version:
 Severity:  Normal  | Resolution:
 Keywords:  |  Actual Points:
Parent ID:  | Points:
 Reviewer:  irl |Sponsor:
+-
Changes (by irl):

 * status:  needs_review => merge_ready


Comment:

 Ok, checks and tests pass, I've not actually run the code but I did look
 over it before Brussels in more detail and it looked good then. I don't
 see any reason to delay this deployment (except don't do it 5pm Friday
 because weekend).

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29166 [Metrics/Statistics]: Run modules from Java only

[Tor Bug Tracker & Wiki]

#29166: Run modules from Java only
+--
 Reporter:  karsten |  Owner:  karsten
 Type:  enhancement | Status:  needs_review
 Priority:  Medium  |  Milestone:
Component:  Metrics/Statistics  |Version:
 Severity:  Normal  | Resolution:
 Keywords:  |  Actual Points:
Parent ID:  | Points:
 Reviewer:  irl |Sponsor:
+--

Comment (by karsten):

 Replying to [comment:5 karsten]:
 > Replying to [comment:4 notirl]:
 > > In general this looks OK, probably is best to not deploy it just
 before Brussels though. I can give it a more thorough review after
 Brussels.
 >
 > I think that not deploying before Brussels is a fine plan. Maybe we can
 talk about the review ''in'' Brussels and also do the deployment together.

 We briefly talked about this in Brussels and decided that it makes more
 sense for (not)irl to do another review and some local tests after
 Brussels.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29166 [Metrics/Statistics]: Run modules from Java only

[Tor Bug Tracker & Wiki]

#29166: Run modules from Java only
+--
 Reporter:  karsten |  Owner:  karsten
 Type:  enhancement | Status:  needs_review
 Priority:  Medium  |  Milestone:
Component:  Metrics/Statistics  |Version:
 Severity:  Normal  | Resolution:
 Keywords:  |  Actual Points:
Parent ID:  | Points:
 Reviewer:  irl |Sponsor:
+--

Comment (by karsten):

 Replying to [comment:4 notirl]:
 > {{{
 > this.connection = DriverManager.getConnection(jdbcString);
 > }}}
 >
 > Should this not still be explicitly `this.jdbcString`? It's still a
 class member and not a local variable.

 It's indeed a class variable now, not an instance variable anymore, so we
 cannot use `this.` anymore. Of course, we could argue whether an instance
 variable might still make more sense. I didn't pay too much attention to
 this, because I'm planning to simplify code around database access across
 modules very soon.

 > Some of the paths have been defined with `/` in them, which breaks
 compatibility with non-Unix operating systems. (Windows uses `\` and RISC
 OS uses `:` as examples.) I think we don't care but it's worth being aware
 of it.
 >
 > We should have a policy and be explicit about our use of, or avoidance
 of, java.nio.file.Paths.

 I agree that using `Paths` would be cleaner. I tried to keep `Paths` but
 then realized that it's producing fewer code changes to just switch (back)
 to `File` in some places. Again, this is code where I anticipate major
 changes in the near future. For example, I'd like to have a single place
 in the code that reads descriptors and passes them to the modules, much
 like how Onionoo works. That's going to eliminate a lot of places where we
 had previously used `Paths`.

 > In general this looks OK, probably is best to not deploy it just before
 Brussels though. I can give it a more thorough review after Brussels.

 I think that not deploying before Brussels is a fine plan. Maybe we can
 talk about the review ''in'' Brussels and also do the deployment together.

 Thanks for this initial review!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29166 [Metrics/Statistics]: Run modules from Java only

[Tor Bug Tracker & Wiki]

#29166: Run modules from Java only
+--
 Reporter:  karsten |  Owner:  karsten
 Type:  enhancement | Status:  needs_review
 Priority:  Medium  |  Milestone:
Component:  Metrics/Statistics  |Version:
 Severity:  Normal  | Resolution:
 Keywords:  |  Actual Points:
Parent ID:  | Points:
 Reviewer:  irl |Sponsor:
+--

Comment (by notirl):

 {{{
 this.connection = DriverManager.getConnection(jdbcString);
 }}}

 Should this not still be explicitly `this.jdbcString`? It's still a class
 member and not a local variable.


 

 Some of the paths have been defined with `/` in them, which breaks
 compatibility with non-Unix operating systems. (Windows uses `\` and RISC
 OS uses `:` as examples.) I think we don't care but it's worth being aware
 of it.

 We should have a policy and be explicit about our use of, or avoidance of,
 java.nio.file.Paths.


 

 In general this looks OK, probably is best to not deploy it just before
 Brussels though. I can give it a more thorough review after Brussels.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29166 [Metrics/Statistics]: Run modules from Java only

[Tor Bug Tracker & Wiki]

#29166: Run modules from Java only
+--
 Reporter:  karsten |  Owner:  karsten
 Type:  enhancement | Status:  needs_review
 Priority:  Medium  |  Milestone:
Component:  Metrics/Statistics  |Version:
 Severity:  Normal  | Resolution:
 Keywords:  |  Actual Points:
Parent ID:  | Points:
 Reviewer:  irl |Sponsor:
+--
Changes (by karsten):

 * status:  needs_revision => needs_review
 * reviewer:   => irl


Comment:

 There, revised my earlier patch and assigned irl as reviewer as discussed
 today. Please review that commit above plus
 [https://gitweb.torproject.org/user/karsten/metrics-
 web.git/commit/?h=task-29166=d8049ba7ebf6c2eb353ed2a68124ca87de73a3dc
 commit d8049ba in the same branch]. Thanks!

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29166 [Metrics/Statistics]: Run modules from Java only

[Tor Bug Tracker & Wiki]

#29166: Run modules from Java only
+
 Reporter:  karsten |  Owner:  karsten
 Type:  enhancement | Status:  needs_revision
 Priority:  Medium  |  Milestone:
Component:  Metrics/Statistics  |Version:
 Severity:  Normal  | Resolution:
 Keywords:  |  Actual Points:
Parent ID:  | Points:
 Reviewer:  |Sponsor:
+
Changes (by karsten):

 * status:  needs_review => needs_revision


Comment:

 Taking this out of review until I have resolved all the path issues that I
 just discovered.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

[tor-bugs] #29166 [Metrics/Statistics]: Run modules from Java only

[Tor Bug Tracker & Wiki]

#29166: Run modules from Java only
+--
 Reporter:  karsten |  Owner:  karsten
 Type:  enhancement | Status:  assigned
 Priority:  Medium  |  Milestone:
Component:  Metrics/Statistics  |Version:
 Severity:  Normal  |   Keywords:
Actual Points:  |  Parent ID:
   Points:  |   Reviewer:
  Sponsor:  |
+--
 As discussed earlier, I'd like to run all modules for the daily updater
 from Java rather than using Ant. This is the first step in streamlining
 things and re-using code between modules. Posting a branch in a second.

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs

Re: [tor-bugs] #29166 [Metrics/Statistics]: Run modules from Java only

[Tor Bug Tracker & Wiki]

#29166: Run modules from Java only
+--
 Reporter:  karsten |  Owner:  karsten
 Type:  enhancement | Status:  needs_review
 Priority:  Medium  |  Milestone:
Component:  Metrics/Statistics  |Version:
 Severity:  Normal  | Resolution:
 Keywords:  |  Actual Points:
Parent ID:  | Points:
 Reviewer:  |Sponsor:
+--
Changes (by karsten):

 * status:  assigned => needs_review


Comment:

 Please review [https://gitweb.torproject.org/user/karsten/metrics-
 web.git/commit/?h=task-29166=421e413274485739fcddd23b4c7bd217045984fe
 commit 421e413 in my task-29166 branch].

--
Ticket URL: 
Tor Bug Tracker & Wiki 
The Tor Project: anonymity online
___
tor-bugs mailing list
tor-bugs@lists.torproject.org
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-bugs