Re: [tor-bugs] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+--
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  closed
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:  fixed
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--
Changes (by karsten):

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


Comment:

 The original purpose of this ticket, making Onionoo's classes available to
 other Onionoo clients, can now be accomplished by simply using Onionoo's
 thin jar as dependency. No need to move classes to metrics-lib anymore. I
 made a remark on #24870 regarding the `ZonedDateTime` discussion above.
 Other than that, there's nothing else to be done here. Closing. 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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  needs_revision
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+

Comment (by karsten):

 Alright, cherry-picked e65bb30 and 403f107 and pushed them to master.
 Thanks for the reviews so far!

--
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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  needs_revision
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+
Changes (by irl):

 * status:  needs_review => needs_revision


Comment:

 Ok, we shouldn't spend longer on it than we have to.

 Commit 403f107 looks good to me.

--
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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+--
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--
Changes (by karsten):

 * status:  needs_revision => needs_review


Comment:

 Oh, well. Even though the `ZonedDateTime` change doesn't look terribly
 hard, I'm still running into issues. I feel like this is a perfect-is-the-
 enemy-of-good case. How about we postpone the `ZonedDateTime` change and
 go with the Jackson-related changes only for now? In particular, would you
 like to review
 
[https://gitweb.torproject.org/user/karsten/onionoo.git/commit/?h=task-24296=403f107954c3a35e8b444e401e650d8d10b8f396
 commit 403f107 in my task-24296 branch] that adds annotations as discussed
 above? We can still make more changes later. 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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  needs_revision
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+

Comment (by karsten):

 Replying to [comment:8 irl]:
 > The first commit looks ok, but I'd be happier with an explicit
 annotation on the class to indicate the naming strategy in case Jackson
 decides to change its default.
 >
 > {{{
 > @JsonNaming(PropertyNamingStrategy.SnakeCaseStrategy.class)
 > public class DetailsDocument extends Document {
 >   ...
 > }}}

 Agreed! In fact, we should consider adding explicit annotations for
 everything that we're currently configuring in the `ObjectMapper`.
 Ideally, these classes would be useful even without our class that
 configures an `ObjectMapper`.

 > When you say Jenkins, do you mean Jackson?

 Absolutely!

 > Additional getters and setters would be ok, but I think actually having
 the `ZonedDateTime` representations would be the more elegant solution.
 >
 > Even though not the "recommended" method,
 [[https://stackoverflow.com/a/27952473|this StackOverflow answer]] looks
 to contain an example of doing it in the way I had done it for Gson. This
 would avoid dependency on extra JARs which it looks like is the Jackson
 preferred way to do it.

 Agreed, this looks like an okay way of doing it.

 Thanks for reviewing and commenting! I'll put this back on my list.

--
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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  needs_revision
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+
Changes (by irl):

 * status:  needs_review => needs_revision


Comment:

 The first commit looks ok, but I'd be happier with an explicit annotation
 on the class to indicate the naming strategy in case Jackson decides to
 change its default.

 {{{
 @JsonNaming(PropertyNamingStrategy.SnakeCaseStrategy.class)
 public class DetailsDocument extends Document {
   ...
 }}}

 When you say Jenkins, do you mean Jackson?

 Additional getters and setters would be ok, but I think actually having
 the `ZonedDateTime` representations would be the more elegant solution.

 Even though not the "recommended" method,
 [[https://stackoverflow.com/a/27952473|this StackOverflow answer]] looks
 to contain an example of doing it in the way I had done it for Gson. This
 would avoid dependency on extra JARs which it looks like is the Jackson
 preferred way to do it.

--
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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+--
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--

Comment (by karsten):

 Replying to [comment:6 karsten]:
 > Mote that I spent quite some time on the `ZonedDateTime` change
 mentioned above. Maybe we'll have to do that after moving things over to
 metrics-lib.

 s/Mote/Note/

 Sometimes it helps to sleep over something. I guess my mistake was that I
 wanted to change attribute types from `String` to `ZonedDateTime`, and
 that turned out to be more difficult with Jenkins than I had expected.
 What we could do, though, is leave attributes as `String` types and simply
 provide additional getters and setters that work with `ZonedDateTime`
 representations of those `String` values. I might try that out later
 today, though not now.

 Leaving in needs_review for the first commit that is unrelated to dates.

--
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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+--
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  needs_review
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--
Changes (by karsten):

 * status:  new => needs_review


Comment:

 I made a start here, but I'm not sure whether I'll be able to continue
 working on this in the remainder of this week. Would you mind reviewing a
 first commit that prepares this change and that seems useful to do anyway?
 It's in
 
[https://gitweb.torproject.org/user/karsten/onionoo.git/commit/?h=task-24296=e65bb3095bad404433c0224960f2891e5a399323
 commit e65bb30 in my task-24296 branch]. Thanks!

 Mote that I spent quite some time on the `ZonedDateTime` change mentioned
 above. Maybe we'll have to do that after moving things over to metrics-
 lib.

--
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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+--
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--

Comment (by irl):

 Replying to [comment:4 irl]:
 > Replying to [comment:3 karsten]:
 > > Hmm, the downside of having an enum like that is that we'll have to
 put out a new metrics-lib release whenever there's a new flag. I'll think
 more about this.
 >
 > Is this a lot of work?

 Aaaah, I see. It would need to know about the flag even before we want to
 do anything with it, because it would appear in the consensus. In this
 case it would not be a good idea to have this as an automatic
 serialization/deserialization.

 If something else starts using the images then we can think about this
 again, but for now we should not include the enum.

--
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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+--
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--

Comment (by irl):

 Replying to [comment:3 karsten]:
 > Hmm, the downside of having an enum like that is that we'll have to put
 out a new metrics-lib release whenever there's a new flag. I'll think more
 about this.

 Is this a lot of work?

--
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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+--
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--

Comment (by karsten):

 Replying to [comment:2 irl]:
 > Possibly `org.torproject.metrics.bot.tor.ZonedDateTimeDeserializer`
 (although this was written for Gson, not Jackson). This is useful to
 provide a consistent date/time parsing to avoid having to parse it in
 every consumer of the documents.

 Okay, I'm not sure how to do this in Jackson, but I'll look into using
 `ZonedDateTime` rather than `String` for timestamps.

 > I would like to have `org.torproject.metrics.bot.tor.RelayFlag` in
 metrics-lib, with a function to return these from details documents. I
 didn't write a deserializer for these yet though and it would also mean
 that the flags images would need to be included in metrics-lib.

 Hmm, the downside of having an enum like that is that we'll have to put
 out a new metrics-lib release whenever there's a new flag. I'll think more
 about this.

 > There are also the
 `org.torproject.metrics.bot.tor.{BaseRelay,Bridge,Relay}` interfaces that
 could be included in metrics-lib. The `generateBadge()` methods can be
 ignored, I'll move these to seperate badge classes.

 Yes, I was planning to include interfaces like `RelaySummaryDocument`
 containing all fields in a relay details object, and the same for
 `BridgeSummaryDocument`, `RelayDetailsDocument`, etc. I'll take a look at
 those metrics-bot interfaces while writing those for metrics-lib.

 Alright, I'll give this a try and let you know when I have something to
 look at. 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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+--
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--

Comment (by irl):

 Possibly `org.torproject.metrics.bot.tor.ZonedDateTimeDeserializer`
 (although this was written for Gson, not Jackson). This is useful to
 provide a consistent date/time parsing to avoid having to parse it in
 every consumer of the documents.

 I would like to have `org.torproject.metrics.bot.tor.RelayFlag` in
 metrics-lib, with a function to return these from details documents. I
 didn't write a deserializer for these yet though and it would also mean
 that the flags images would need to be included in metrics-lib.

 There are also the
 `org.torproject.metrics.bot.tor.{BaseRelay,Bridge,Relay}` interfaces that
 could be included in metrics-lib. The `generateBadge()` methods can be
 ignored, I'll move these to seperate badge classes.

--
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] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+--
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   | Resolution:
 Keywords:   |  Actual Points:
Parent ID:   | Points:
 Reviewer:   |Sponsor:
-+--

Comment (by karsten):

 I made a start here in preparation of resolving #24295 afterwards.

 But before I write more code, is there code in metrics-bot that I should
 be looking at first? Something that is worth reusing? Any insights from
 writing that code that we should keep in mind when writing this new code?

 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

[tor-bugs] #24296 [Metrics/Onionoo]: Make Onionoo's document classes available as part of metrics-lib

[Tor Bug Tracker & Wiki]

#24296: Make Onionoo's document classes available as part of metrics-lib
-+--
 Reporter:  karsten  |  Owner:  metrics-team
 Type:  enhancement  | Status:  new
 Priority:  Medium   |  Milestone:
Component:  Metrics/Onionoo  |Version:
 Severity:  Normal   |   Keywords:
Actual Points:   |  Parent ID:
   Points:   |   Reviewer:
  Sponsor:   |
-+--
 This ticket is based on a discussion on #24036:

  - At some point in the future we may want to move parts of
 `org.torproject.onionoo.docs` to metrics-lib, so that metrics-bot and
 other clients can re-use that code. More specifically, we should only move
 the externally provided JSON objects, not the internally used status
 objects.

--
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