Verify consensus download scheduling works properly

nskinkel / oppy

A Tor client (onion proxy) implementation in Python

BSD 3-Clause "New" or "Revised" License

24 stars 3 forks source link

Verify consensus download scheduling works properly #2

Closed nskinkel closed 10 years ago

nskinkel commented 10 years ago

I'm currently using reactor.callLater to schedule the next lookup (using a function in the same file).

Questions:

If I schedule this by importing the reactor in this file and using a local (to this module) function, will that just work across the rest of the program?
What happens if someone tries to access the consensus while doing a download? Does Twisted provide a lock? Do we need to be using deferreds here?

Tagging as a bug because this needs testing.

nskinkel commented 10 years ago

We should write this project in general in a way that's a "unit-testable" as possible. This will be somewhat difficult since a lot of components will depend on the system being in various states, but we should try to write as many parts to be purely functional as possible. Testing this specific part of NetStatus should be integrated into overall unittests for the package.

dwtj commented 10 years ago

I definitely agree with your points about making things "unit-testable". (This is a good idea, even if we do not always get around to actually making the corresponding unit tests.)

NetStatus SHOULD support local file-backed cache. So, how about we keep a few network status consensus documents around just to instantiate the database for the purposes of unit testing?

dwtj commented 10 years ago

Concerning your first question about reactor importing:

I do believe that there will be only one reactor in our process. I also believe that in order to use and interact with this reactor from our other python modules (i.e. other .py files imported into our process) we need only import reactor again in these modules.

Nik and I also found out about the possibility of importing the reactor within a smaller scope (i.e. not at the module level), but I think that we should avoid doing so. (Furthermore, I believe that that came from krondo's tutorial, and it seems like the kind of thing that might have become out of date.)

dwtj commented 10 years ago

In the case that "someone tries to access the consensus while doing a download", I believe that we do indeed want to be using deferreds here. I think that this use of deferreds will be somewhat different from other uses, because we are essentially rolling our own (potentially) blocking source of input.

nskinkel commented 10 years ago

Fixed in 2cfefbee7f (thanks David)