[Twisted-Python] "twistd" in Twisted 16.4.0 can't import modules/packages from current working directory
Hi all I couldn't find Twisted-specific group, so posting here. Recently Twisted 16.4.0 got released. Yesterday I've tried to upgrade it for my apps and got an error. I've created simple example, which demonstrates it. File service.tac: ======================================= import os from twisted.application import service, internet import mymodule application = service.Application("Demo application") ---------------------------------------- File mymodule.py: ======================================== def myfunction(asd): """ Stub function """ ---------------------------------------- If you try to run it with twistd -y service.tac you'll get an error: == output ============================== Unhandled Error Traceback (most recent call last): File "/usr/local/lib/python2.7/site-packages/twisted/application/app.py", line 648, in run runApp(config) File "/usr/local/lib/python2.7/site-packages/twisted/scripts/twistd.py", line 25, in runApp _SomeApplicationRunner(config).run() File "/usr/local/lib/python2.7/site-packages/twisted/application/app.py", line 379, in run self.application = self.createOrGetApplication() File "/usr/local/lib/python2.7/site-packages/twisted/application/app.py", line 444, in createOrGetApplication application = getApplication(self.config, passphrase) --- <exception caught here> --- File "/usr/local/lib/python2.7/site-packages/twisted/application/app.py", line 455, in getApplication application = service.loadApplication(filename, style, passphrase) File "/usr/local/lib/python2.7/site-packages/twisted/application/service.py", line 411, in loadApplication passphrase) File "/usr/local/lib/python2.7/site-packages/twisted/persisted/sob.py", line 223, in loadValueFromFile eval(codeObj, d, d) File "service.tac", line 7, in <module> import mymodule exceptions.ImportError: No module named mymodule Failed to load application: No module named mymodule ---------------------------------------- The errors comes down to this: twistd script does not add current working directory to python path (or removes it, I don't know what exactly happens), so it fails to import any packages/modules from it. The issue does not appear in previous version (Twisted 16.3.2). Any ideas what caused it?
On Thu, Sep 01, 2016 at 08:42:02PM +0700, Yuri wrote:
The errors comes down to this: twistd script does not add current working directory to python path (or removes it, I don't know what exactly happens), so it fails to import any packages/modules from it. The issue does not appear in previous version (Twisted 16.3.2).
Any ideas what caused it?
The commits that closed this ticket: http://twistedmatrix.com/trac/ticket/8491 Generally: https://github.com/twisted/twisted/commit/c5a4c635de259cd1b92a555c930aa42616... Specifically: https://github.com/twisted/twisted/commit/c5a4c635de259cd1b92a555c930aa42616... The bin/twistd script would add the current working directory to sys.path prior to running the actual twistd logic. This was not ported over. I'm going to say that was a conscious decision, as it coincided with moving twisted to a src/ layout and a fair bit of discussion (visible in that ticket) about how these changes have made trial only discover installed code and not whatever's in the current working directory. To a developer of Twisted it's clear that the project's trying to replace magical code discovery with boring code importing. To a user, that's not clear, and we need to clearly document this. Being a Twisted developer, I do think in general that it's better to install your code into an existing sys.path entry instead of adding a sys.path entry that contains your code. Would this be difficult for your application, and if so, why? If it's possible to make it easier for your application I'd like to help. Thanks for using Twisted! -Mark
On 1 Sep 2016, at 21:42, Yuri <yuri_abzyanov@fastmail.fm> wrote:
Hi all
I couldn't find Twisted-specific group, so posting here.
Recently Twisted 16.4.0 got released. Yesterday I've tried to upgrade it for my apps and got an error.
...
The errors comes down to this: twistd script does not add current working directory to python path (or removes it, I don't know what exactly happens), so it fails to import any packages/modules from it. The issue does not appear in previous version (Twisted 16.3.2).
Any ideas what caused it?
Yes -- we moved to using setuptools console scripts, and these console scripts don't add "." to the PYTHONPATH. We realised this in prerelease but decided against fixing it, as it adding the current working dir to the PATH has lead to a lot of subtle bugs in the past and this is a good chance to make a break from them. So, in short, this is expected behaviour -- we generally want people to be running twistd, trial, etc on *installed* Python packages -- testing or running from checkouts often hides many bugs about what is or isn't included in the installed package by accident. If you rely on this behaviour, though, set the PYTHONPATH environment variable to "." -- e.g. `env PYTHONPATH=. twistd -n myplugin`. - Amber
On Thu, Sep 1, 2016 at 11:36 AM, Amber "Hawkie" Brown < hawkowl@atleastfornow.net> wrote:
On 1 Sep 2016, at 21:42, Yuri <yuri_abzyanov@fastmail.fm> wrote:
Hi all
I couldn't find Twisted-specific group, so posting here.
Recently Twisted 16.4.0 got released. Yesterday I've tried to upgrade it for my apps and got an error.
...
The errors comes down to this: twistd script does not add current working directory to python path (or removes it, I don't know what exactly happens), so it fails to import any packages/modules from it. The issue does not appear in previous version (Twisted 16.3.2).
Any ideas what caused it?
Yes -- we moved to using setuptools console scripts, and these console scripts don't add "." to the PYTHONPATH. We realised this in prerelease but decided against fixing it, as it adding the current working dir to the PATH has lead to a lot of subtle bugs in the past and this is a good chance to make a break from them.
So, in short, this is expected behaviour -- we generally want people to be running twistd, trial, etc on *installed* Python packages -- testing or running from checkouts often hides many bugs about what is or isn't included in the installed package by accident. If you rely on this behaviour, though, set the PYTHONPATH environment variable to "." -- e.g. `env PYTHONPATH=. twistd -n myplugin`.
FWIW, as a user, it would have been nice to have a NEWS entry for this. It would have been easier to discover there than searching through the mail archives. Thanks, Jean-Paul
On Jan 2, 2017, at 6:10 AM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
On Thu, Sep 1, 2016 at 11:36 AM, Amber "Hawkie" Brown <hawkowl@atleastfornow.net <mailto:hawkowl@atleastfornow.net>> wrote:
On 1 Sep 2016, at 21:42, Yuri <yuri_abzyanov@fastmail.fm <mailto:yuri_abzyanov@fastmail.fm>> wrote:
Hi all
I couldn't find Twisted-specific group, so posting here.
Recently Twisted 16.4.0 got released. Yesterday I've tried to upgrade it for my apps and got an error.
...
The errors comes down to this: twistd script does not add current working directory to python path (or removes it, I don't know what exactly happens), so it fails to import any packages/modules from it. The issue does not appear in previous version (Twisted 16.3.2).
Any ideas what caused it?
Yes -- we moved to using setuptools console scripts, and these console scripts don't add "." to the PYTHONPATH. We realised this in prerelease but decided against fixing it, as it adding the current working dir to the PATH has lead to a lot of subtle bugs in the past and this is a good chance to make a break from them.
So, in short, this is expected behaviour -- we generally want people to be running twistd, trial, etc on *installed* Python packages -- testing or running from checkouts often hides many bugs about what is or isn't included in the installed package by accident. If you rely on this behaviour, though, set the PYTHONPATH environment variable to "." -- e.g. `env PYTHONPATH=. twistd -n myplugin`.
FWIW, as a user, it would have been nice to have a NEWS entry for this. It would have been easier to discover there than searching through the mail archives.
Yes, there definitely should have been. We only realized the implications of the change after the fact, and only after some discussion decided that it was in fact desirable and should not be rolled back in a patch release. We don't have a process for fixing NEWS files right now, and given that releases are immutable, I wonder if there's any way we could. So, in the rare case that we mess up this way in the future, what (aside from the mailing list) would be a good communication mechanism to users? -glyph
On 4 Jan 2017, at 07:20, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Jan 2, 2017, at 6:10 AM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
On Thu, Sep 1, 2016 at 11:36 AM, Amber "Hawkie" Brown <hawkowl@atleastfornow.net <mailto:hawkowl@atleastfornow.net>> wrote:
On 1 Sep 2016, at 21:42, Yuri <yuri_abzyanov@fastmail.fm <mailto:yuri_abzyanov@fastmail.fm>> wrote:
Hi all
I couldn't find Twisted-specific group, so posting here.
Recently Twisted 16.4.0 got released. Yesterday I've tried to upgrade it for my apps and got an error.
...
The errors comes down to this: twistd script does not add current working directory to python path (or removes it, I don't know what exactly happens), so it fails to import any packages/modules from it. The issue does not appear in previous version (Twisted 16.3.2).
Any ideas what caused it?
Yes -- we moved to using setuptools console scripts, and these console scripts don't add "." to the PYTHONPATH. We realised this in prerelease but decided against fixing it, as it adding the current working dir to the PATH has lead to a lot of subtle bugs in the past and this is a good chance to make a break from them.
So, in short, this is expected behaviour -- we generally want people to be running twistd, trial, etc on *installed* Python packages -- testing or running from checkouts often hides many bugs about what is or isn't included in the installed package by accident. If you rely on this behaviour, though, set the PYTHONPATH environment variable to "." -- e.g. `env PYTHONPATH=. twistd -n myplugin`.
FWIW, as a user, it would have been nice to have a NEWS entry for this. It would have been easier to discover there than searching through the mail archives.
Yes, there definitely should have been. We only realized the implications of the change after the fact, and only after some discussion decided that it was in fact desirable and should not be rolled back in a patch release.
We don't have a process for fixing NEWS files right now, and given that releases are immutable, I wonder if there's any way we could. So, in the rare case that we mess up this way in the future, what (aside from the mailing list) would be a good communication mechanism to users?
-glyph
We have, in the past, fixed up historic NEWS files in later releases (e.g. the one which removed 2.6 support). We could always roll these changes into a post1, but, that seems like a lot of effort. Maybe we could put errata on the blog? - Amber
On Tue, Jan 3, 2017 at 3:22 PM, Amber "Hawkie" Brown < hawkowl@atleastfornow.net> wrote:
We have, in the past, fixed up historic NEWS files in later releases (e.g. the one which removed 2.6 support). We could always roll these changes into a post1, but, that seems like a lot of effort. Maybe we could put errata on the blog?
With a link to the blog post in the NEWS file? Cold checking blog was below "read git history and try to correlate with the issue tracker" on my list of how to track down this change. Jean-Paul
Note there is still some confusion over this matter. See < http://twistedmatrix.com/trac/ticket/8972>, < http://twistedmatrix.com/trac/ticket/8978>, and < https://github.com/twisted/twisted/pull/672>. Jean-Paul On Tue, Jan 3, 2017 at 3:24 PM, Jean-Paul Calderone < exarkun@twistedmatrix.com> wrote:
On Tue, Jan 3, 2017 at 3:22 PM, Amber "Hawkie" Brown < hawkowl@atleastfornow.net> wrote:
We have, in the past, fixed up historic NEWS files in later releases (e.g. the one which removed 2.6 support). We could always roll these changes into a post1, but, that seems like a lot of effort. Maybe we could put errata on the blog?
With a link to the blog post in the NEWS file? Cold checking blog was below "read git history and try to correlate with the issue tracker" on my list of how to track down this change.
Jean-Paul
Thanks for highlighting those. I've put the link in the other direction as well.
On Jan 12, 2017, at 3:42 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
Note there is still some confusion over this matter. See <http://twistedmatrix.com/trac/ticket/8972 <http://twistedmatrix.com/trac/ticket/8972>>, <http://twistedmatrix.com/trac/ticket/8978 <http://twistedmatrix.com/trac/ticket/8978>>, and <https://github.com/twisted/twisted/pull/672 <https://github.com/twisted/twisted/pull/672>>.
Jean-Paul
On Tue, Jan 3, 2017 at 3:24 PM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote: On Tue, Jan 3, 2017 at 3:22 PM, Amber "Hawkie" Brown <hawkowl@atleastfornow.net <mailto:hawkowl@atleastfornow.net>> wrote:
We have, in the past, fixed up historic NEWS files in later releases (e.g. the one which removed 2.6 support). We could always roll these changes into a post1, but, that seems like a lot of effort. Maybe we could put errata on the blog?
With a link to the blog post in the NEWS file? Cold checking blog was below "read git history and try to correlate with the issue tracker" on my list of how to track down this change.
Jean-Paul
_______________________________________________ Twisted-Python mailing list Twisted-Python@twistedmatrix.com http://twistedmatrix.com/cgi-bin/mailman/listinfo/twisted-python
On Fri, Jan 13, 2017 at 1:13 AM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
Thanks for highlighting those. I've put the link in the other direction as well.
Craig seems eager to go ahead with reverting this change in behavior. https://github.com/twisted/twisted/pull/672#issuecomment-275956265 As far as I can tell, no one has weighed in on the other side. So I'm inclined to go along with the reversion. Jean-Paul
On Feb 7, 2017, at 6:59 AM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
On Fri, Jan 13, 2017 at 1:13 AM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote: Thanks for highlighting those. I've put the link in the other direction as well.
Craig seems eager to go ahead with reverting this change in behavior.
https://github.com/twisted/twisted/pull/672#issuecomment-275956265 <https://github.com/twisted/twisted/pull/672#issuecomment-275956265>
As far as I can tell, no one has weighed in on the other side. So I'm inclined to go along with the reversion.
My 2¢ for the other side is: if trial does this, but twist and twistd don't, then it will be possible to get a passing test run for a plugin that doesn't get loaded. I think it would be simpler and easier to debug to leave these consistent. (I really don't think we should put this behavior back into twist or twistd, because "cd malware; twist web --path ." should not pwn your machine.) -glyph
On Tue, Feb 7, 2017 at 11:29 AM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
On Feb 7, 2017, at 6:59 AM, Jean-Paul Calderone <exarkun@twistedmatrix.com> wrote:
On Fri, Jan 13, 2017 at 1:13 AM, Glyph Lefkowitz <glyph@twistedmatrix.com> wrote:
Thanks for highlighting those. I've put the link in the other direction as well.
Craig seems eager to go ahead with reverting this change in behavior.
https://github.com/twisted/twisted/pull/672#issuecomment-275956265
As far as I can tell, no one has weighed in on the other side. So I'm inclined to go along with the reversion.
My 2¢ for the other side is: if trial does this, but twist and twistd don't, then it will be possible to get a passing test run for a plugin that doesn't get loaded. I think it would be simpler and easier to debug to leave these consistent.
This is an interesting corner case, but I think the twistd and twist issues should be pursued in separate discussions and tickets. For trial, I would like to proceed with https://github.com/twisted/twisted/pull/672/ . With my conversion of trial to a console script, the new behavior was unintentional on my part. Since all the unit tests passed, I did not notice. In this ticket: https://twistedmatrix.com/trac/ticket/8978 Job-Evers-Meltzer provided a use-case where the new behavior of trial broke existing usage. trial can be used as a general-purpose tool for running unittest-style tests, much like pytest or nose. The new behavior is disruptive, for minimal benefit, so I would like to restore the old behavior. I added a unit test for this, so in future, if this breaks again, we will catch it. -- Craig
On Feb 12, 2017, at 11:08 AM, Craig Rodrigues <rodrigc@crodrigues.org> wrote:
On Tue, Feb 7, 2017 at 11:29 AM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote:
On Feb 7, 2017, at 6:59 AM, Jean-Paul Calderone <exarkun@twistedmatrix.com <mailto:exarkun@twistedmatrix.com>> wrote:
On Fri, Jan 13, 2017 at 1:13 AM, Glyph Lefkowitz <glyph@twistedmatrix.com <mailto:glyph@twistedmatrix.com>> wrote: Thanks for highlighting those. I've put the link in the other direction as well.
Craig seems eager to go ahead with reverting this change in behavior.
https://github.com/twisted/twisted/pull/672#issuecomment-275956265 <https://github.com/twisted/twisted/pull/672#issuecomment-275956265>
As far as I can tell, no one has weighed in on the other side. So I'm inclined to go along with the reversion.
My 2¢ for the other side is: if trial does this, but twist and twistd don't, then it will be possible to get a passing test run for a plugin that doesn't get loaded. I think it would be simpler and easier to debug to leave these consistent.
This is an interesting corner case, but I think the twistd and twist issues should be pursued in separate discussions and tickets.
For trial, I would like to proceed with https://github.com/twisted/twisted/pull/672/ <https://github.com/twisted/twisted/pull/672/> . With my conversion of trial to a console script, the new behavior was unintentional on my part. Since all the unit tests passed, I did not notice. In this ticket: https://twistedmatrix.com/trac/ticket/8978 <https://twistedmatrix.com/trac/ticket/8978> Job-Evers-Meltzer provided a use-case where the new behavior of trial broke existing usage.
trial can be used as a general-purpose tool for running unittest-style tests, much like pytest or nose. The new behavior is disruptive, for minimal benefit, so I would like to restore the old behavior.
I added a unit test for this, so in future, if this breaks again, we will catch it.
There's a lot of controversy around this type of organization of tests; personally, I believe it is a horrible antipattern, others, notably Donald Stufft, (wrongly) regard it as a best practice. Apropos of the comment I put on 9035, https://twistedmatrix.com/trac/ticket/9035#comment:4 <https://twistedmatrix.com/trac/ticket/9035#comment:4>, would it be acceptable for Job Evers‐Meltzer if the syntax were simply 'trial ./tests/' instead? -glyph
Thanks for swift response! I do not tend to install my applications. Never did that to be honest, and don't know what kind of issues to expect. Definitely should try it. For now workaround will be fine. Just wanted to confirm that it's an intended behavior and find out the reasons why that was changed.
participants (6)
-
Amber "Hawkie" Brown -
Craig Rodrigues -
Glyph Lefkowitz -
Jean-Paul Calderone -
Mark Williams -
Yuri