[Toaster] [review-request] adamian/20150707_bugs
Damian, Alexandru
alexandru.damian at intel.com
Thu Jul 23 06:02:30 PDT 2015
Hello,
Revised patchset pushed to the same branch. This patchset passes the HTML5
conformance tests, and fixes 500 page issues seen on the previous version.
Can you please review and submit upstream when possible ?
Cheers,
Alex
On Wed, Jul 22, 2015 at 3:44 PM, Damian, Alexandru <
alexandru.damian at intel.com> wrote:
> 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
>
--
Alex Damian
Yocto Project
SSG / OTC
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/toaster/attachments/20150723/96059eb8/attachment-0001.html>
More information about the toaster
mailing list