On Tue, Jul 21, 2015 at 1:39 PM, Michael Wood <[email protected]> wrote:
> On 21/07/15 12:26, Damian, Alexandru wrote: > >> Hi, >> >> I am reluctant to use a check_output() call because I would like the >> stderroroutput in the exception message as to debug easily what went wrong. >> >> > I'm not sure I follow, the CalledProcessError exception generated by a > non-zero exist status on check_output has all the data you'd need in it. > > The CalledProcessError has the return code, but not the stderr output. Manually raising the exception allows me to get the stderr. > > The check_output() documentation specifies to use Popen with >> communicate() in order to properly get the stderr output/ >> >> Note >> >> Do not use stderr=PIPE with this function as that can deadlock based on >> the child process error volume. UsePopen < >> https://docs.python.org/2/library/subprocess.html#subprocess.Popen> with >> the communicate() method when you need a stderr pipe. >> >> >> >> If everything else is fine, can you please submit this patchset upstream ? >> >> Thank you, >> Alex >> >> On Fri, Jul 17, 2015 at 11:27 AM, Michael Wood <[email protected] >> <mailto:[email protected]>> wrote: >> >> >> >> From 4c34c2261685ca45a0013ed331a9663bd5e15898 Mon Sep 17 00:00:00 2001 >> >> diff --git >> a/bitbake/lib/toaster/bldcontrol/management/commands/checksettings.py >> b/bitbake/lib/toaster/bldcontrol/management/commands/checksettings.py >> index 9b591ad..590a7b0 100644 >> --- >> a/bitbake/lib/toaster/bldcontrol/management/commands/checksettings.py >> +++ >> b/bitbake/lib/toaster/bldcontrol/management/commands/checksettings.py >> @@ -166,7 +166,12 @@ class Command(NoArgsCommand): >> for dirname in >> self._recursive_list_directories(be.sourcedir,2): >> if os.path.exists(os.path.join(dirname, >> ".templateconf")): >> import subprocess >> - conffilepath, error = >> subprocess.Popen('bash -c ". '+os.path.join(dirname, >> ".templateconf")+'; echo \"\$TEMPLATECONF\""', shell=True, >> stdout=subprocess.PIPE).communicate() >> + proc = subprocess.Popen('bash -c ". >> '+os.path.join(dirname, ".templateconf")+'; echo >> \"\$TEMPLATECONF\""', shell=True, stdout=subprocess.PIPE) >> + conffilepath, stderroroutput = >> proc.communicate() >> + proc.wait() >> + if proc.returncode != 0: >> + raise Exception("Failed to source >> TEMPLATECONF: %s" % stderroroutput) >> + >> >> >> Can't we just use subprocess.check_output and a try/except for all >> of this? >> >> Apart from that everything else looks fine. >> >> Michael >> >> >> On 15/07/15 13:31, Damian, Alexandru wrote: >> >> Hi, >> >> The patch that Michael suggested to fix is now refactored, and >> the patchset rebased. >> >> I pushed the changes on the same branch (force update). >> >> Can you please review ? >> >> Cheers, >> Alex >> >> On Mon, Jul 13, 2015 at 5:30 PM, Michael Wood >> <[email protected] <mailto:[email protected]> >> <mailto:[email protected] >> <mailto:[email protected]>>> wrote: >> >> >> Could you include the summary of each of the commits for >> review so >> that we can be sure which patches are the new ones >> >> * 3b0073f bitbake: toaster: fix updates on failed build >> requests >> * 1bdd687 bitbake: toaster: replace raising Exceptions in >> loadconf >> * 99a626a bitbake: toaster: do not stop data import on bad >> data >> >> Look fine to me >> >> * 4fc46e7 bitbake: toastergui: fixing pylint warnings >> >> This one needs splitting up, pylint fixes, logging fixes , >> string >> to int casting, etc >> >> Thanks, >> >> Michael >> >> >> >> On 08/07/15 15:52, Barros Pena, Belen wrote: >> >> >> On 07/07/2015 17:27, "Damian, Alexandru" >> <[email protected] >> <mailto:[email protected]> >> <mailto:[email protected] >> <mailto:[email protected]>>> >> wrote: >> >> Hi, >> >> >> I have some comments below, >> >> >> Alex >> >> On Tue, Jul 7, 2015 at 3:54 PM, Barros Pena, Belen >> <[email protected] >> <mailto:[email protected]> >> <mailto:[email protected] >> <mailto:[email protected]>>> wrote: >> >> >> >> On 07/07/2015 15:10, "Damian, Alexandru" >> <[email protected] >> <mailto:[email protected]> >> <mailto:[email protected] >> >> <mailto:[email protected]>>> >> >> wrote: >> >> Hello, >> >> >> I'm pushing two patches for review >> >> >> - one is fixing 7955 >> >> Thanks for the quick fix, Alex! I've tested this on >> master, and I've >> noticed a couple of things: >> >> 1. The invalid data from the layer index causes >> warnings >> when importing >> the information. This might be because the debug >> mode is >> enabled, though, >> but I thought I'd bringing it up just in case. >> >> This happens because data doesn't match what Toaster >> expect - Toaster >> has a bit stricter requirements than Layer Index. >> I would >> expect that >> the warning messages are helpful to the user, and they >> should not be >> obscured. If a particular user wishes to not see >> some of >> the messages, >> they can set up the debug level to something higher in >> settings.py. Or we >> can ship with a higher debug level by default, but >> I don't >> think we >> should silently ignore bad data. >> >> Sure: I can see your point. I was only bringing it up, >> but I >> am not sure >> what's best, to be honest: showing them or not. As you >> said, >> falling >> silent doesn't sound right; on the other hand, the >> messages >> look a bit >> alarming. >> >> >> 2. I can see at least one recipe in the 'all recipes' >> table without a >> name. This particular one is provided by meta-ivi. >> You can >> add the layer >> and you get a build button, which you can click, >> although >> when you do so >> the build does not seem to start. I think we need >> to hide >> any recipes that >> do not have a name from the list. They are >> invalid, and >> should not be >> exposed to users. >> >> We can add such a check, of course, on imported >> data. I >> would say this >> is the object of a different bug report, though. >> >> Done! >> https://bugzilla.yoctoproject.org/show_bug.cgi?id=7969 >> >> Also, I think we are going to need to back port >> the fix to >> Fido. >> >> Is back porting ok? Should we track this somehow? >> >> Thanks! >> >> Belén >> >> Cheers >> >> Belén >> >> - one is fixing various issues highlighted by >> pylint >> >> >> >> https://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=adamian/20 >> 1 >> 50707_bugs >> >> >> >> Can you please review ? >> >> >> Cheers, >> Alex >> >> >> >> >> -- >> Alex Damian >> Yocto Project >> >> SSG / OTC >> >> >> >> >> >> >> >> >> >> >> >> >> -- Alex Damian >> Yocto Project >> >> SSG / OTC >> >> >> >> >> >> -- _______________________________________________ >> toaster mailing list >> [email protected] <mailto:[email protected]> >> <mailto:[email protected] >> >> <mailto:[email protected]>> >> https://lists.yoctoproject.org/listinfo/toaster >> >> >> >> >> -- Alex Damian >> Yocto Project >> SSG / OTC >> >> >> --------------------------------------------------------------------- >> Intel Corporation (UK) Limited >> Registered No. 1134945 (England) >> Registered Office: Pipers Way, Swindon SN3 1RJ >> VAT No: 860 2173 47 >> >> This e-mail and any attachments may contain confidential >> material for >> the sole use of the intended recipient(s). Any review or >> distribution >> by others is strictly prohibited. If you are not the intended >> recipient, please contact the sender and delete all copies. >> >> >> --------------------------------------------------------------------- >> Intel Corporation (UK) Limited >> Registered No. 1134945 (England) >> Registered Office: Pipers Way, Swindon SN3 1RJ >> VAT No: 860 2173 47 >> >> This e-mail and any attachments may contain confidential material for >> the sole use of the intended recipient(s). Any review or distribution >> by others is strictly prohibited. If you are not the intended >> recipient, please contact the sender and delete all copies. >> >> >> >> >> -- >> Alex Damian >> Yocto Project >> SSG / OTC >> >> --------------------------------------------------------------------- >> Intel Corporation (UK) Limited >> Registered No. 1134945 (England) >> Registered Office: Pipers Way, Swindon SN3 1RJ >> VAT No: 860 2173 47 >> >> This e-mail and any attachments may contain confidential material for >> the sole use of the intended recipient(s). Any review or distribution >> by others is strictly prohibited. If you are not the intended >> recipient, please contact the sender and delete all copies. >> >> > --------------------------------------------------------------------- > Intel Corporation (UK) Limited > Registered No. 1134945 (England) > Registered Office: Pipers Way, Swindon SN3 1RJ > VAT No: 860 2173 47 > > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. > -- Alex Damian Yocto Project SSG / OTC
-- _______________________________________________ toaster mailing list [email protected] https://lists.yoctoproject.org/listinfo/toaster
