Opened at 2011-09-22T20:05:33Z
Last modified at 2014-10-27T01:22:40Z
#1539 closed enhancement
stop putting pkg_resources.require() into .tac files — at Version 11
| Reported by: | zooko | Owned by: | davidsarah |
|---|---|---|---|
| Priority: | major | Milestone: | 1.10.1 |
| Component: | code-nodeadmin | Version: | 1.9.0a1 |
| Keywords: | forward-compatibility appname tac packaging setuptools test-needed | Cc: | |
| Launchpad Bug: |
Description (last modified by zooko)
One thing that we can do right away to ease this is change create-node to stop putting the call to pkg_resources.require() in the tahoe-client.tac.
Change History (12)
comment:1 Changed at 2011-09-24T00:13:12Z by davidsarah
- Milestone changed from undecided to 1.10.0
- Owner set to davidsarah
- Status changed from new to assigned
comment:2 follow-up: ↓ 3 Changed at 2011-09-24T03:06:12Z by zooko
There are two calls to pkg_resources.require() in the .tac files that we currently produce. According to 1aed9fcfa1c45538 the patch that added them, this was necessary to make them importable if they were installed in setuptools's "multiversion mode". I don't think we need to support that. We experimented with it (see #530) and removing multiversion mode from our setup.py did not break any of our unit tests, not even test-with-fake-pkg or test-with-fake-dists.
comment:3 in reply to: ↑ 2 Changed at 2011-09-24T03:48:24Z by davidsarah
Replying to zooko:
There are two calls to pkg_resources.require() in the .tac files that we currently produce. According to 1aed9fcfa1c45538 the patch that added them, this was necessary to make them importable if they were installed in setuptools's "multiversion mode".
Those calls should never have been necessary even for multiversion mode; I think the commit comment was wrong. pkg_resources.require() does not, despite the misleading name and documentation, make a package available that wouldn't already have been available. The set of packages available in the working set is determined at the point of first importing pkg_resources, which happens in the setuptools-generated support script, long before the .tac is executed.
So I guess I'm violently agreeing with your conclusion that these calls aren't necessary ;-)
comment:4 Changed at 2012-04-01T03:55:22Z by davidsarah
- Milestone changed from 1.11.0 to 1.10.0
comment:5 Changed at 2012-05-23T04:59:13Z by warner
- Keywords needs-review added
something like that?
comment:6 Changed at 2012-05-23T15:43:43Z by davidsarah
- Description modified (diff)
- Keywords review-needed added; needs-review removed
- Status changed from assigned to new
Will review.
Note that import pkg_resources has side effects on sys.path even if pkg_resources.require() is not used; so, removing the import might have an additional effect. I can't immediately remember whether that's desirable or not; need to page this stuff back into my brain.
comment:7 Changed at 2012-05-23T15:43:52Z by davidsarah
- Status changed from new to assigned
comment:8 Changed at 2012-05-23T16:24:51Z by zooko
According to 1aed9fcfa1c45538, I added that so that it would find an allmydata and twisted package tht had been installed in "multi-version mode". I guess that mode of install offered by setuptools doesn't put the installed egg into the sys.path at *all* until a pkg_resources.require() with a matching version number has been invoked. We experimented with it at one point as a way to work around other problems, but I believe we gave up on using it.
comment:9 Changed at 2012-09-04T16:11:13Z by davidsarah
Obsolete if we accept #1159.
comment:10 Changed at 2013-03-21T18:44:48Z by warner
- Milestone changed from 1.10.0 to 1.11.0
I want this, but won't block 1.10 for it. Kicking it and its cousin #1159 into 1.11.
comment:11 Changed at 2013-08-28T15:58:55Z by zooko
- Description modified (diff)
- Milestone changed from soon to 1.11.0
Agreed.