[tor-dev] Tor Export

[Damian Johnson]

Hi Erik. In addition to Norman's feedback...

> def descriptor_csv_exp(descr, head=True, incl_fields=[], excl_fields=[]):

It's redundant to call this descriptor_csv_exp() since it's in
'stem.descriptor.*'. Lets just call it something simple like
export_csv(). Also, I still think that we should combine the
descriptor_csv_exp() and descriptors_csv_exp() in a similar fashion to
the reader and controller methods.

> doc_loc = os.path.expanduser('~') + '/torexport.csv'
> doc = open(doc_loc, 'w')

Obviously something to change. :)

We should have a function that returns us a csv string, and maybe
another that writes it to a file instead.

> # define incl_fields, 4 cases where final case is incl_fields already
> # defined and excl_fields left blank, so no action is necessary.
> if not incl_fields and excl_fields:
>   incl_fields = attr.keys()
>   for f in excl_fields:
>     if f in incl_fields:
>       incl_fields.remove(f)

Alternatively you could just default incl_fields and excl_fields earlier...

if not descrs: return ""

# default to including all of the parameters
if incl_fields is None:
  incl_fields = vars(descrs[0]).keys()

if excl_fields is None:
  excl_fields = []

Also, lets fully spell these out (descriptors, include_fields,
exclude_fields) since this is exposed to our user via the keyword
parameter.

> final = {}
> for at in attr:
>   if at in incl_fields:
>     final[at] = attr[at]
> dwriter.writerow(final)

You're relying on each of your vars(desr) calls to provide the same
ordering which, while probably safe, isn't something that we should
rely on. Lets iterate over incl_fields instead.

Also, you aren't checking that all of the descriptors are of the same
type. If you get a list with both server descriptors and extrainfo
descriptors then I'm not sure what this function will do, but it's
probably not what the user wants. Please include that use case while
writing unit tests.

For the future I would suggest opening a ticket on trac and asking for
a code review there. This is what Ravi does and it seems to work
pretty well - here's an example...
https://trac.torproject.org/6114

Cheers! -Damian