[tor-bugs] #1837 [BridgeDB]: bridgedb learns to load file of which bridges are blocked where

[Tor Bug Tracker & Wiki]

#1837: bridgedb learns to load file of which bridges are blocked where
----------------------+-----------------------------------------------------
 Reporter:  arma      |          Owner:              
     Type:  defect    |         Status:  needs_review
 Priority:  normal    |      Milestone:              
Component:  BridgeDB  |        Version:              
 Keywords:            |         Parent:  #1608       
   Points:            |   Actualpoints:              
----------------------+-----------------------------------------------------

Comment(by karsten):

 Code looks good.  A few comments:

  - Can you rebase this branch to origin/master, too?

  - What's the format of the `blocked-bridges` file?  I think this should
 go into the bridge spec (which is not in origin/master, yet).

  - I found that you're removing blocked bridges from the result, similar
 to the /16 filtering.  I'm not sure if this is the right thing to do.
 Maybe it is.  I'm adding this comment mostly for myself to think about it
 when looking at your branch again.

  - You have the line `self.db = db = bridgedb.Storage.getDB()` in both
 BridgeBlock and CountryBlock.  I think BridgeBlock doesn't need it.  Also,
 what's the `db` in the middle doing?

  - Can you add two log statements for successfully loading and failing to
 load the GeoIP database?

  - You have at least two instances of `SELECT * FROM` in your code where
 you reference columns by their index.  This is fragile, because someone
 might overlook these places when changing the schema.  Better put the
 column names in here.  Also, is the `(fingerprint,)` and `(countryCode,)`
 syntax correct?

  - How would I migrate the schema from version 1 to 2?  Can you add a
 sentence or two to the README for that?

 Thanks!

-- 
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/1837#comment:5>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online