[tor-bugs] #9277 [BridgeDB]: BridgeDB's handling of config files is non-persistent and uses an old-style class
Tor Bug Tracker & Wiki
blackhole at torproject.org
Thu Aug 1 01:19:34 UTC 2013
#9277: BridgeDB's handling of config files is non-persistent and uses an old-style
class
----------------------+-----------------------------------------------------
Reporter: isis | Owner: isis
Type: defect | Status: needs_review
Priority: normal | Milestone:
Component: BridgeDB | Version:
Keywords: | Parent:
Points: | Actualpoints:
----------------------+-----------------------------------------------------
Changes (by isis):
* status: needs_revision => needs_review
Comment:
Replying to [comment:2 cypherpunks]:
> On first pass this looks quite good.
Thanks for reviewing!
> - (9e9ddc56e2f2bd98ff) I think the commits will be more comprehendible
if each commit message describes what the commit does, instead of
describing the patchset in the first commit (this may already be on your
TODO list, though).
Yes, I agree. The commit messages were originally that way, but because
this branch and the logging branch were intertwined for #9199 and then
split and then rebased again several times I got seriously annoyed with
messing with git commit logs, decided it was a waste of my time, and
copied what I wanted to say into a file, and then back into the first
commit message on each branch.
> - In Main.py:reconfigure(), can you add the type of `configuration` to
the docstring?
Sure thing.
{{{
* commit 9241c624aaf0d0220f3602fbbc9682a2bbc6a47c
| gpg: Signature made Thu 01 Aug 2013 00:00:38 UTC
gpg: using RSA key A3ADB67A2CDB8B35
gpg: Good signature from "Isis! <isis at patternsinthevoid.net>" [ultimate]
gpg: aka "Isis <isis at leap.se>" [ultimate]
gpg: aka "Isis <isis at torproject.org>" [ultimate]
gpg: aka "Isis! <isis at riseup.net>" [ultimate]
gpg: Signature notation:
isis at patternsinthevoid.net=0A6A58A14B5946ABDE18E207A3ADB67A2CDB8B35
gpg: Signature expires Fri 01 Aug 2014 00:00:38 UTC
Author: Isis Lovecruft <isis at torproject.org>
| AuthorDate: 78 minutes ago
| Commit: Isis Lovecruft <isis at torproject.org>
| CommitDate: 78 minutes ago
|
| Update bridgedb.Main.reconfigure docstring with configuration
parameter type.
|
| * ADD :type: and :param: info on configuration kwarg for
| bridgedb.Main.reconfigure() function.
| ---
| lib/bridgedb/Main.py | 5 +++++
| 1 file changed, 5 insertions(+)
|
| diff --git a/lib/bridgedb/Main.py b/lib/bridgedb/Main.py
| index fb94f2a..a0b89e5 100644
| --- a/lib/bridgedb/Main.py
| +++ b/lib/bridgedb/Main.py
| @@ -222,6 +222,11 @@ def reconfigure(configuration=None):
| configuration file, and apply those settings, and then, if extra
settings
| were given in :attr:`bridgedb.config.TESTING_CONFIG`, apply those
settings
| on top of the settings from the configuration file.
| +
| + :type configuration: :class:`bridgedb.config.Conf` or dict
| + :param configuration: A previously created
:class:`bridgedb.config.Conf`
| + object, or a Python dictionary containing configuration
key/value
| + settings.
| """
| options, arguments = Opt.parseOpts()
| settings = {}
}}}
> - I don't fully understand Conf.load(). I think the method is ok, but
it's unique. Can you explain why the config file should "not ending with a
'c'"?
> {{{
> + elif isinstance(config_file, (ModuleType, self.__class__)):
> + if ((hasattr(config_file, 'file') is not None) and
> + not config_file.file.endswith('c')):
> + self.file = config_file.file
> }}}
Added an explanation:
{{{
* commit 68c75cb06630677544e86408a6ebefbe4991dab9 (HEAD, tpo-
isis/fix/9277-config, isislovecruft/fix/9277-config,
greyarea/fix/9277-config, fix/9277-config)
| gpg: Signature made Thu 01 Aug 2013 00:09:15 UTC
gpg: using RSA key A3ADB67A2CDB8B35
gpg: Good signature from "Isis! <isis at patternsinthevoid.net>" [ultimate]
gpg: aka "Isis <isis at leap.se>" [ultimate]
gpg: aka "Isis <isis at torproject.org>" [ultimate]
gpg: aka "Isis! <isis at riseup.net>" [ultimate]
gpg: Signature notation:
isis at patternsinthevoid.net=0A6A58A14B5946ABDE18E207A3ADB67A2CDB8B35
gpg: Signature expires Fri 01 Aug 2014 00:09:15 UTC
Author: Isis Lovecruft <isis at torproject.org>
| AuthorDate: 66 minutes ago
| Commit: Isis Lovecruft <isis at torproject.org>
| CommitDate: 66 minutes ago
|
| Add comment in bridgedb.config.Conf.load() explaining odd checks.
|
| * Explain the `if not config_file.endswith('c') line of code,
tl;dr: we don't
| want to load compiled Python binary files, even if they otherwise
seem like
| a Python object.
| ---
| lib/bridgedb/config.py | 6 ++++++
| 1 file changed, 6 insertions(+)
|
| diff --git a/lib/bridgedb/config.py b/lib/bridgedb/config.py
| index ebafe3a..34496ee 100644
| --- a/lib/bridgedb/config.py
| +++ b/lib/bridgedb/config.py
| @@ -143,6 +143,12 @@ class Conf(dict):
| else:
| self.file = os.path.abspath(config_file)
| elif isinstance(config_file, (ModuleType, self.__class__)):
| + ## Because the config file is not technically a "flat
file", but
| + ## instead is a file containing Python global variables,
when it
| + ## is loaded it is treated as a Python source file. This
means it
| + ## gets compiled, and Python's idiot interpreter adds a 'c'
to the
| + ## extension of any compiled Python binary. We don't want
to load
| + ## binary files.
| if ((hasattr(config_file, 'file') is not None) and
| not config_file.file.endswith('c')):
| self.file = config_file.file
}}}
> - Should this use self.file instead of config_file? (config.py:150,
config.py:157)
> {{{
> + logging.info("Loading config file: %s" % config_file)
> ...
> + logging.err(err, "Loading config file '%s' failed!" %
config_file)
> }}}
Effectively, they often end up being the same thing. When they are
different, ```config_file``` is just ```self.file``` after the last '/':
{{{
(bridgedb)∃!isisⒶwintermute:(fix/9277-config *)~/code/torproject/bridgedb
∴ python ./lib/TorBridgeDB.py -t
> /home/isis/code/torproject/bridgedb/lib/bridgedb/config.py(159)load()
-> logging.info("Loading config file: %s" % config_file)
(Pdb) print config_file
./bridgedb.conf
(Pdb) print self.file
./bridgedb.conf
}}}
--
Ticket URL: <https://trac.torproject.org/projects/tor/ticket/9277#comment:3>
Tor Bug Tracker & Wiki <https://trac.torproject.org/>
The Tor Project: anonymity online
More information about the tor-bugs
mailing list