meejah / txtorcon

Twisted-based asynchronous Tor control protocol implementation. Includes unit-tests, examples, state-tracking code and configuration abstraction.
http://fjblvrw2jrxnhtg67qpbzi45r7ofojaoo3orzykesly2j3c2m3htapid.onion/
MIT License
250 stars 72 forks source link

Implement a get_version() method #108

Closed sambuddhabasu closed 9 years ago

sambuddhabasu commented 9 years ago

Txtorcon can make use of a get_version() method which will call get_info('version'). If the users also have stem installed, the version will be in the format stem.version.Version. The version is constructed from strings that conform to the 'new' style in the tor version-spec and so, can be compared too.

sambuddhabasu commented 9 years ago

@meejah I am currently working on this issue

meejah commented 9 years ago

Instead of changing the API, is it possible to keep it the same an have TorControlProtocol.version simply be a Stem object? (Of course, only if the user requested using stem, and it's installed).

That is, the API is currently "proto.version" which doesn't return a Deferred because we grab the version during bootstrap().

sambuddhabasu commented 9 years ago

Yes, it the TorControlProtocol.version can be made as a Stem object. As you mentioned that the version is grabbed during _bootstrap(), in which function call does the user specify that stem should be used or not? Should this be added as an extra parameter in _bootstrap() itself or some other method?

meejah commented 9 years ago

probably to TorControlProtocol's constructor, as use_stem=False by default and set a parameter; the _bootstrap method isn't part of the API and not supposed to be called by "user code".

Ultimately, this would need to filter to the create_tor_connection-type factory methods too (or, it could maybe be automatic there based on imports, but I think TorControlProtocol should take it as a kwarg in any case).

sambuddhabasu commented 9 years ago

The use_stem=False will be taken in by the TorControlProtocol's constructor as a kwarg. While connecting to a running instance of Tor by using build_tor_connection(), the user can specify whether or not they want the use of stem. We also need to specify the use of stem in the other modules of txtorcon also based on the user requirements. Do we achieve this by defining TorContolProtocol.use_stem?

sambuddhabasu commented 9 years ago

Or the use of stem can also be added to __init__.py and can be defined under __all__. This way, the whole of txtorcon can make use of the same variable by using txtorcon.use_stem However, if the users want the use of stem only for some specific functions, this way is not desirable. Please let me know what are your thoughts about this?

meejah commented 9 years ago

for now, stick with the member in TorControlProtocol; I think anything in other modules that might want to know about stem usage will also need a protocol instance to be useful...