Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#18719 closed enhancement (implemented)

provide executable jar

Reported by: iwakeh Owned by: iwakeh
Priority: Medium Milestone: CollecTor 1.0.0
Component: Metrics/CollecTor Version:
Severity: Normal Keywords: ctip
Cc: karsten Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

metrics-db currently has (at least) six classes providing a main method, which are invoked using six corresponding ant tasks.

One solution for providing an executable jar:

Add a new class org.torproject.ernie.db.Main as the jar's main class adding the following functionality:

  • calling java -jar metrics-db.jar will show a usage message and exit.
  • calling java -jar metrics-db.jar <taskname> will run the main method of the class called in the current ant task of the same name, i.e., taskname is one of
    • bridgedescs
    • bridgepools
    • exitlists
    • relaydescs
    • torperf
    • updateindex

Finally, remove the then obsolete ant tasks from build.xml.

What is missing?
Should a task be added or removed?
Other thoughts?

Child Tickets

Attachments (1)

0001-Provide-executable-jar-containing-all-dependencies-r.patch (8.4 KB) - added by iwakeh 3 years ago.
implements this task

Download all attachments as: .zip

Change History (14)

comment:1 Changed 3 years ago by iwakeh

Status: newneeds_information

comment:2 Changed 3 years ago by karsten

Sounds like a fine plan. Two suggestions:

  • I'd like to move over to a package hierarchy org.torproject.collector* and get rid of org.torproject.ernie* packages and classes. How about we use that new package structure for new classes like the new Main class you're thinking of? That could be org.torproject.collector.Main.
  • Can you also update the scripts in bin/* and call the new Main method with appropriate task name and ideally parameters like -Xmx2048m as in the current build.xml?

Other than that, nothing comes to mind that would be missing.

comment:3 in reply to:  2 Changed 3 years ago by iwakeh

Replying to karsten:

  • I'd like to move over to a package hierarchy org.torproject.collector* and get rid of org.torproject.ernie* packages and classes. How about we use that new package structure for new classes like the new Main class you're thinking of? That could be org.torproject.collector.Main.

org.torproject.collector.Main is fine. (also see #18727)

  • Can you also update the scripts in bin/* and call the new Main method with appropriate task name and ideally parameters like -Xmx2048m as in the current build.xml?

Sure. Good point.

Last edited 3 years ago by iwakeh (previous) (diff)

comment:4 Changed 3 years ago by iwakeh

depends on #18727 and on #18792 and now #18794

Last edited 3 years ago by iwakeh (previous) (diff)

comment:5 Changed 3 years ago by iwakeh

Discuss the following question from #18792 comment 3, by karsten:

  • Should the produced .jar file contain the required .jar files from lib/, except for example junit.jar? I don't remember the reasons for/against doing that in Onionoo and it might be that the answer is no for good reasons. And should we remove the compiled test classes? See Onionoo's build.xml where we're doing that.

comment:6 in reply to:  5 Changed 3 years ago by iwakeh

Status: needs_informationassigned

Onionoo includes the classes from the required jars (cf. jar task)
(It is not possible to add jars inside a jar to the classpath. That's why we used zipgroupfileset in Onionoo.)

For CollecTor the jar collector.jar will also include the necessary classes.

comment:7 Changed 3 years ago by iwakeh

Status: assignedneeds_review

Please find attached a patch implementing this task.

This could have also be achieved by using annotations, but as there will probably only one additional class with a main method, I decided to use a simpler approach. Only one added class (as outlined above) is necessary that way.

The shell scripts are changed accordingly and the jar ant task is extended to include all dependencies and the Main-Class property.

Main.java is free of checkstyle complaints.

comment:8 Changed 3 years ago by karsten

Status: needs_reviewneeds_revision

Looks good! Just one thing: Can you change <zipgroupfileset> to just include the libraries that are necessary for running CollecTor? Two reasons: the .jar file will be smaller and it won't contain classes from packages that are not in Debian stable. Other than that, ready to merge. Thanks!

Changed 3 years ago by iwakeh

implements this task

comment:9 Changed 3 years ago by iwakeh

Status: needs_revisionneeds_review

Oops, thanks for noticing the oversized runtime jar!

The patch is replaced and the resulting runtime jar is reduced by a factor of ten.

comment:10 Changed 3 years ago by karsten

Status: needs_reviewmerge_ready

Thanks for making that change. For some reason I'm unable to apply your patch using git am. Would you mind pushing your branch to your shiny new git.torproject.org repository?

comment:12 Changed 3 years ago by karsten

Resolution: implemented
Status: merge_readyclosed

Great, merged to master!

In the future, consider pushing your changes to a separate task branch, like task-18719 (or whatever naming scheme you prefer). The downsides of using your master branch is that it may derive from the official master and that you can only have one change under review.

Closing this ticket as implemented. Thanks!

comment:13 Changed 3 years ago by iwakeh

Milestone: CollecTor 1.0.0

Added to milestone for first release.

Note: See TracTickets for help on using tickets.