Opened 6 years ago

Closed 3 years ago

#12085 closed task (wontfix)

Develop a python wrapper for onionoo services

Reported by: sreenatha Owned by:
Priority: Medium Milestone:
Component: Metrics/Tor Weather Version:
Severity: Keywords: weather-rewrite
Cc: karsten, meejah Actual Points:
Parent ID: Points:
Reviewer: Sponsor:

Description

Initial work started from the discussion at https://lists.torproject.org/pipermail/tor-dev/2014-March/006603.html

Although the wrapper is mainly intended for usage in rewriting weather.torproject.org, it should be flexible enough to support other applications as well.

Child Tickets

Change History (5)

comment:1 Changed 6 years ago by karsten

Component: OnionooTor Weather

sreenatha and I discussed that we should first write something we need for Weather, and afterwards consider making it more general so that it can be re-used by other projects. Changing the component to Tor Weather.

comment:2 Changed 6 years ago by sreenatha

Status: newneeds_review

https://github.com/lucyd/tor-weather/tree/onionoo-wrapper/weather/onionoo_wrapper

After looking at Onion-Py, I made an attempt at developing the onionoo wrapper. Here is a short introduction on how to use it to fetch onionoo documents.

from onionoo_wrapper import *

# Instantiate an OnionooRequest object
req = objects.OnionooRequest()

# Specify the document to be fetched from onionoo
document_type = 'details'
parameters = {
    'type': 'relay',
    'fields': ['nickname', 'fingerprint']
}

# Build the request
req.build_request(document_type, params=parameters)

# Get the response as an OnionooResponse object
resp = req.get_response()
print resp.status_code, resp.document

# Build request for another document
document_type = 'summary'
parameters = {
    'type': 'relay',
    'running': 'true'
}
req.build_request(document_type, parameters)

# Get the response again as an OnionooResponse object
resp = req.get_response()
Last edited 6 years ago by sreenatha (previous) (diff)

comment:3 Changed 6 years ago by meejah

This was originally in an email thread; adding to trac for posterity.

Hi Sreenatha,

I'm taking a look through the code now, and making some notes /
comments. Should I just put this on Trac (in future)? I can't recall
that we talked specifically about code-review type stuff.

Perhaps it should be on tor-dev if Trac isn't appropriate?
Anyway.

Overall, the code reads nice and straight-forward at first glance
through.

I like the use of memcached for keeping documents around!

It would be nice to have some docstrings on at least some of the
methods. For example, I don't understand the str type-checking
vs. .encode('utf-8') json serialzation code (as in: why?) That said, I'm
certainly not suggesting EVERYTHING needs a docstring -- "RelaySummary"
conveys enough "documentation" by itself ;)

I would force Python2 to use new-style objects everywhere (that is, all
classes should derive from object, like "class Foo(object):"

There should also be some unit-tests for this code. It's good practice
to write tests. It is important in dynamic languages like Python to run
every line just to make sure you don't have a typo, etc.

You're using "requests"! Great :)

I see that many of the classes are basically "just" copying (and
possibly also re-arranging) attributes and so forth from "document"
objects. This seems like a lot of duplicated/"boilerplate" code and it
would be really nice to reduce this. One idea might be to create a
base-class that can simply be told what attributes to "forward", and use
a getattr overide (or maybe @property decorators) to accomplish
this. Something like:

class _DocumentForwarder(object):

doc = None # set by subclass, the document instance
keys = dict() # from attribute name to document.get() name
def getattr(self, k):

if k in self.dict:

return self.dict[k]

# we purposely let KeyError propagate up, if thrown
doc_key = self.keys[k]
# this lets us do "keysfoo? = None" in subclasses instead
# of "keysfoo? = 'foo'"
if not doc_key:

doc_key = k

return self.doc.get(doc_key)


class RelaySummary(_DocumentForwarder):

def init(self, document):

self.doc = document
self.keys = dict(nickname='n', fingerprint='f', addresses='a', running='r')

import unittest
class Test(unittest.TestCase):

def test_simple(self):

mydoc = dict(n='blam', f='floom', a='amamamamama', r='ruuuuuuuuuuun')
x = RelaySummary(mydoc)
self.assertEqual(x.nickname, 'blam')
self.assertEqual(x.fingerprint, 'floom')
self.assertRaises(KeyError, lambda: x.n)

On the other hand, it kind of seems like you're simply providing
attribute-style access to the "document" dict, with some multi-level
objects in a few spots (and possibly renaming a some things). I would
ask whether this really gains anything over just returning the document,
or wrapping it directly with attribute-style access provider
instances. Questions that might help answer this would be:

  1. Will you need to know what type of document you have after calling

get_response()? If code like "if type(resp) is Summary" can be avoided,
I think that's A Very Good Thing Indeed :)

  1. Could we just run a couple renaming operations over the dict we get

back from Onionoo, instead? If this is just convenience, that might work
fine...

  1. Are the names of things in the Onionoo documents well-known (e.g. to

Onionoo users, or tor-metrics-people)? If so, it might be better for the
people most-expected to read this code to see the "real" document
attribute names (attribute-style access or not).

Also, if it's kept like it is, you should probably acknowledge in the
docstring that it started from code by Lukas Erlacher (duk3luk3).

This is probably more just a style-thing, but like the requests library,
would a method-based API for "do a request to onionoo please" maybe work
more naturally? I personally find "build an object, change the object,
access result from object" less straightfoward than a method that takes
the args it needs, and returns the result.

I guess I'm saying a module-level method "get(request_instance)" would
return a OnionooResponse... or even just "get(a, few, args)" would do
that, and there's no Request object at all. If the code using this will
do many of the very same (or really similar) requests, than perhaps
having a Request object might make sense.

thanks,
meejah

comment:4 Changed 5 years ago by sreenatha

So here is the updated onionoo-wrapper after making more changes. Check out weather/weatherapp/onionoo_wrapper.

https://github.com/lucyd/tor-weather/tree/onionoo-wrapper

  • Added some utilities for clients like the t-shirt, hourly scripts that operate on the response data.
  • Temporarily removed the caching feature on meejah's suggestion. Will add it later after finishing up the backend scripts before actual deployment.
  • Wrote a couple of unit tests using mock.

Please suggest additional utility methods or test scenarios if I missed any.

comment:5 Changed 3 years ago by karsten

Resolution: wontfix
Status: needs_reviewclosed

Tor Weather has been discontinued as of May 24, 2016: https://lists.torproject.org/pipermail/tor-relays/2016-June/009424.html. Batch-closing all remaining tickets as announced in #19382. A list of these tickets and any other Weather tickets modified after June 26, 2016 will be available here: https://trac.torproject.org/projects/tor/query?changetime=Jun+27%2C+2016..&component=^Metrics%2FTor+Weather

Note: See TracTickets for help on using tickets.