[Toaster] [review-request] adamian/20150707_bugs
Damian, Alexandru
alexandru.damian at intel.com
Wed Jul 22 07:44:57 PDT 2015
Please hold on submitting this branch, spotted bug in the submission queue.
Cheers,
Alex
On Tue, Jul 21, 2015 at 1:44 PM, Damian, Alexandru <
alexandru.damian at intel.com> wrote:
>
>
> On Tue, Jul 21, 2015 at 1:39 PM, Michael Wood <michael.g.wood at intel.com>
> 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 <michael.g.wood at intel.com
>>> <mailto:michael.g.wood at intel.com>> 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
>>> <michael.g.wood at intel.com <mailto:michael.g.wood at intel.com>
>>> <mailto:michael.g.wood at intel.com
>>> <mailto:michael.g.wood at 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 at intel.com
>>> <mailto:alexandru.damian at intel.com>
>>> <mailto:alexandru.damian at intel.com
>>> <mailto:alexandru.damian at 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 at intel.com
>>> <mailto:belen.barros.pena at intel.com>
>>> <mailto:belen.barros.pena at intel.com
>>> <mailto:belen.barros.pena at intel.com>>> wrote:
>>>
>>>
>>>
>>> On 07/07/2015 15:10, "Damian, Alexandru"
>>> <alexandru.damian at intel.com
>>> <mailto:alexandru.damian at intel.com>
>>> <mailto:alexandru.damian at intel.com
>>>
>>> <mailto:alexandru.damian at 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 at yoctoproject.org <mailto:toaster at yoctoproject.org>
>>> <mailto:toaster at yoctoproject.org
>>>
>>> <mailto:toaster at 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.
>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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
>
--
Alex Damian
Yocto Project
SSG / OTC
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/toaster/attachments/20150722/88bd4076/attachment-0001.html>
More information about the toaster
mailing list