[Toaster] [review-request] adamian/20150707_bugs
Damian, Alexandru
alexandru.damian at intel.com
Fri Jul 24 08:06:31 PDT 2015
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
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.yoctoproject.org/pipermail/toaster/attachments/20150724/3e2f8bd1/attachment-0001.html>
More information about the toaster
mailing list