-
Hey, I noticed that fassembler:zeo wasn't alerting the user when the zopectl scripts exited non-zero. I fixed this in http://trac.openplans.org/openplans/changeset/16065 ... but there are a lot of other places where fassembler runs subprocesses and ignores their exit codes. Ignoring subprocess failure is pretty bad, since the build will seem to succeed but potentially we'd have much more confusing runtime problems. I think we should usually bail out and show the user whatever was on stderr, as I did in r16065. But I'm not sure the best way to generalize this. One simple approach would be to use everywhere a Popen subclass something like: class PopenOrBust(subprocess.Popen): def communicate(self): stdout, stderr = super(PopenOrBust).communicate() if self.returncode: raise Exception("command exited %d. From its stderr:\n%s" % (self.returncode, stderr)) Yes? No? Maybe? -- Paul Winkler http://www.openplans.org/people/slinkp/profile yahoo: slinkp23 AIM: slinkp1970 -
I hate the name, and the use of a bare exception class, but everything else I'm +1 on. Jeff On Tue, April 22, 2008 17:08, Paul Winkler wrote: > Hey, > > > I noticed that fassembler:zeo wasn't alerting the user when the > zopectl scripts exited non-zero. I fixed this in > http://trac.openplans.org/openplans/changeset/16065 > ... but there are a lot of other places where fassembler runs > subprocesses and ignores their exit codes. > > Ignoring subprocess failure is pretty bad, since the build will seem > to succeed but potentially we'd have much more confusing runtime problems. > I think we should usually bail out and show the user > whatever was on stderr, as I did in r16065. > > But I'm not sure the best way to generalize this. One simple approach > would be to use everywhere a Popen subclass something like: > > class PopenOrBust(subprocess.Popen): > > def communicate(self): stdout, stderr = super(PopenOrBust).communicate() if > self.returncode: > raise Exception("command exited %d. From its stderr:\n%s" % > (self.returncode, stderr)) > > > > Yes? No? Maybe? > > > -- > > > Paul Winkler > http://www.openplans.org/people/slinkp/profile > yahoo: slinkp23 > AIM: slinkp1970 > > > > -- > Archive: > http://www.openplans.org/projects/fassembler/lists/fassembler-discussion/ > archive/2008/04/1208898540853 To unsubscribe send an email with subject > unsubscribe to fassembler-discussion@.... Please contact > fassembler-discussion-manager@... for questions. > > > !DSPAM:4014,480e545b102852085621377! > > > -
Paul Winkler wrote: > Hey, > > I noticed that fassembler:zeo wasn't alerting the user when the > zopectl scripts exited non-zero. > I fixed this in http://trac.openplans.org/openplans/changeset/16065 > ... but there are a lot of other places where fassembler runs > subprocesses and ignores their exit codes. > > Ignoring subprocess failure is pretty bad, since the build will seem > to succeed but potentially we'd have much more confusing runtime > problems. I think we should usually bail out and show the user > whatever was on stderr, as I did in r16065. > > But I'm not sure the best way to generalize this. One simple approach > would be to use everywhere a Popen subclass something like: > > class PopenOrBust(subprocess.Popen): > > def communicate(self): > stdout, stderr = super(PopenOrBust).communicate() > if self.returncode: > raise Exception("command exited %d. From its stderr:\n%s" % > (self.returncode, stderr)) Huh... something weird is happening. Shouldn't filemaker:667 handle this? Or, is the problem that not enough places are using the maker.run_command method? Ian-
On Tue, Apr 22, 2008 at 04:14:37PM -0500, Ian Bicking wrote: > Huh... something weird is happening. Shouldn't filemaker:667 handle this? > Or, is the problem that not enough places are using the maker.run_command > method? I would've used it if I knew about it! topp_opencore.py only uses run_command once; it uses subprocess.Popen eight times. From glancing at svn blame, my guess is that you did a few things that way because you were writing functions that weren't in the scope of either a maker or a task, so run_command wasn't available; then presumably other people followed your lead. -- Paul Winkler http://www.openplans.org/people/slinkp/profile yahoo: slinkp23 AIM: slinkp1970
-
ok, we're using maker.run_command() a bit more as of r16092. Also added some error checking in make_tarball(); i'd like to know if/when patches fail. One odd thing I've discovered, now that fassembler correctly bails out on add_openplans.py errors: If you wipe your var/zeo, eg. to get a and re-run fassembler:zeo to get a clean dev database, you MUST also re-run fassembler:opencore. I haven't looked into it, but if you don't, the line user = app.acl_users.getUser(admin_id) ... sets the user to None, and then the script barfs on the next line. - PW On Tue, Apr 22, 2008 at 05:35:07PM -0400, Paul Winkler wrote: > On Tue, Apr 22, 2008 at 04:14:37PM -0500, Ian Bicking wrote: > > Huh... something weird is happening. Shouldn't filemaker:667 handle this? > > Or, is the problem that not enough places are using the maker.run_command > > method? > > I would've used it if I knew about it! topp_opencore.py only uses > run_command once; it uses subprocess.Popen eight times. From glancing > at svn blame, my guess is that you did a few things that way because > you were writing functions that weren't in the scope of either a maker > or a task, so run_command wasn't available; then presumably other > people followed your lead. > > -- > > Paul Winkler > http://www.openplans.org/people/slinkp/profile > yahoo: slinkp23 > AIM: slinkp1970 > -- Paul Winkler http://www.openplans.org/people/slinkp/profile yahoo: slinkp23 AIM: slinkp1970
-
-
-
How about giving the user the choice as to what to do, as what happens for many other errors? On Apr 22, 2008, at 5:08 PM, Paul Winkler wrote: > Hey, > > I noticed that fassembler:zeo wasn't alerting the user when the > zopectl scripts exited non-zero. > I fixed this in http://trac.openplans.org/openplans/changeset/16065 > ... but there are a lot of other places where fassembler runs > subprocesses and ignores their exit codes. > > Ignoring subprocess failure is pretty bad, since the build will seem > to succeed but potentially we'd have much more confusing runtime > problems. I think we should usually bail out and show the user > whatever was on stderr, as I did in r16065. > > But I'm not sure the best way to generalize this. One simple approach > would be to use everywhere a Popen subclass something like: > > class PopenOrBust(subprocess.Popen): > > def communicate(self): > stdout, stderr = super(PopenOrBust).communicate() > if self.returncode: > raise Exception("command exited %d. From its stderr:\n%s" % > (self.returncode, stderr)) > > > Yes? No? Maybe? > > -- > > Paul Winkler > http://www.openplans.org/people/slinkp/profile > yahoo: slinkp23 > AIM: slinkp1970 > > > -- > Archive: http://www.openplans.org/projects/fassembler/lists/fassembler-discussion/archive/2008/04/1208898540853 > To unsubscribe send an email with subject unsubscribe to fassembler-discussion@... > . Please contact fassembler-discussion-manager@... > for questions. > > > !DSPAM:4037,480e546c102971096210785! >-
Fassembler does that already, see handle_exception() in filemaker.py. - PW On Tue, Apr 22, 2008 at 05:15:42PM -0400, Douglas Mayle wrote: > How about giving the user the choice as to what to do, as what happens for > many other errors? > > On Apr 22, 2008, at 5:08 PM, Paul Winkler wrote: > >> Hey, >> >> I noticed that fassembler:zeo wasn't alerting the user when the >> zopectl scripts exited non-zero. >> I fixed this in http://trac.openplans.org/openplans/changeset/16065 >> ... but there are a lot of other places where fassembler runs >> subprocesses and ignores their exit codes. >> >> Ignoring subprocess failure is pretty bad, since the build will seem >> to succeed but potentially we'd have much more confusing runtime >> problems. I think we should usually bail out and show the user >> whatever was on stderr, as I did in r16065. >> >> But I'm not sure the best way to generalize this. One simple approach >> would be to use everywhere a Popen subclass something like: >> >> class PopenOrBust(subprocess.Popen): >> >> def communicate(self): >> stdout, stderr = super(PopenOrBust).communicate() >> if self.returncode: >> raise Exception("command exited %d. From its stderr:\n%s" % >> (self.returncode, stderr)) >> >> >> Yes? No? Maybe? >> >> -- >> >> Paul Winkler >> http://www.openplans.org/people/slinkp/profile >> yahoo: slinkp23 >> AIM: slinkp1970 >> >> >> -- >> Archive: >> http://www.openplans.org/projects/fassembler/lists/fassembler-discussion/archive/2008/04/1208898540853 >> To unsubscribe send an email with subject unsubscribe to >> fassembler-discussion@.... Please contact >> fassembler-discussion-manager@... for questions. >> >> >> >> > > > > -- > Archive: > http://www.openplans.org/projects/fassembler/lists/fassembler-discussion/archive/2008/04/1208898945640 > To unsubscribe send an email with subject unsubscribe to > fassembler-discussion@.... Please contact > fassembler-discussion-manager@... for questions. > > > !DSPAM:4043,480e562c106541804284693! > -- Paul Winkler http://www.openplans.org/people/slinkp/profile yahoo: slinkp23 AIM: slinkp1970
-