Re: [review-request] adamian/20150707_bugs

[Michael Wood <michael.g.wood@intel.com>]

 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 
> <michael.g.wood@intel.com <mailto:michael.g.wood@intel.com>> 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"
>         <alexandru.damian@intel.com <mailto:alexandru.damian@intel.com>>
>         wrote:
>
>             Hi,
>
>
>             I have some comments below,
>
>
>             Alex
>
>             On Tue, Jul 7, 2015 at 3:54 PM, Barros Pena, Belen
>             <belen.barros.pena@intel.com
>             <mailto:belen.barros.pena@intel.com>> wrote:
>
>
>
>             On 07/07/2015 15:10, "Damian, Alexandru"
>             <alexandru.damian@intel.com
>             <mailto:alexandru.damian@intel.com>>
>             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
>     toaster@yoctoproject.org <mailto:toaster@yoctoproject.org>
>     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.
>