[Toaster] [review-request] adamian/20150707_bugs
Damian, Alexandru
alexandru.damian at intel.com
Fri Jul 24 08:11:04 PDT 2015
The new patchset lives at:
https://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=adamian/bug_fixes
It is based on Bitbake, not on Poky.
Cheers,
Alex
On Fri, Jul 24, 2015 at 4:06 PM, Damian, Alexandru <
alexandru.damian at intel.com> wrote:
> Hello,
>
> I have rebased the patchset on top of bitbake, for easy merging, and added
> a number of patches:
>
> - fixing pylint issues in the toaster data logger
> - refactoring checksettings.py and fixing toasterconf.json file discovery
> - fixing display of all-builds page (do not show in-progress builds)
>
> Can you please review and submit for merging upstream ?
>
> Cheers,
> Alex
>
>
> On Thu, Jul 23, 2015 at 2:02 PM, Damian, Alexandru <
> alexandru.damian at intel.com> wrote:
>
>> 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
>>
>
>
>
> --
> 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/20150724/8fc3c2d7/attachment-0001.html>
More information about the toaster
mailing list