* [review-request] adamian/20150707_bugs
@ 2015-07-07 14:10 Damian, Alexandru
2015-07-07 14:54 ` Barros Pena, Belen
0 siblings, 1 reply; 16+ messages in thread
From: Damian, Alexandru @ 2015-07-07 14:10 UTC (permalink / raw)
To: toaster@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 310 bytes --]
Hello,
I'm pushing two patches for review
- one is fixing 7955
- one is fixing various issues highlighted by pylint
https://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=adamian/20150707_bugs
Can you please review ?
Cheers,
Alex
--
Alex Damian
Yocto Project
SSG / OTC
[-- Attachment #2: Type: text/html, Size: 1408 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-07 14:10 [review-request] adamian/20150707_bugs Damian, Alexandru
@ 2015-07-07 14:54 ` Barros Pena, Belen
2015-07-07 16:27 ` Damian, Alexandru
0 siblings, 1 reply; 16+ messages in thread
From: Barros Pena, Belen @ 2015-07-07 14:54 UTC (permalink / raw)
To: Damian, Alexandru, toaster@yoctoproject.org
On 07/07/2015 15:10, "Damian, Alexandru" <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.
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.
Also, I think we are going to need to back port the fix to Fido.
Cheers
Belén
>- one is fixing various issues highlighted by pylint
>
>
>https://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=adamian/201
>50707_bugs
>
>
>
>Can you please review ?
>
>
>Cheers,
>Alex
>
>
>
>
>--
>Alex Damian
>Yocto Project
>
>SSG / OTC
>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-07 14:54 ` Barros Pena, Belen
@ 2015-07-07 16:27 ` Damian, Alexandru
2015-07-08 14:52 ` Barros Pena, Belen
0 siblings, 1 reply; 16+ messages in thread
From: Damian, Alexandru @ 2015-07-07 16:27 UTC (permalink / raw)
To: Barros Pena, Belen; +Cc: toaster@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 2140 bytes --]
Hi,
I have some comments below,
Alex
On Tue, Jul 7, 2015 at 3:54 PM, Barros Pena, Belen <
belen.barros.pena@intel.com> wrote:
>
>
> On 07/07/2015 15:10, "Damian, Alexandru" <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.
> 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.
>
> Also, I think we are going to need to back port the fix to Fido.
>
> Cheers
>
> Belén
>
> >- one is fixing various issues highlighted by pylint
> >
> >
> >
> https://git.yoctoproject.org/cgit/cgit.cgi/poky-contrib/log/?h=adamian/201
> >50707_bugs
> >
> >
> >
> >Can you please review ?
> >
> >
> >Cheers,
> >Alex
> >
> >
> >
> >
> >--
> >Alex Damian
> >Yocto Project
> >
> >SSG / OTC
> >
> >
> >
>
>
--
Alex Damian
Yocto Project
SSG / OTC
[-- Attachment #2: Type: text/html, Size: 3833 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* [review-request] adamian/20150707_bugs
@ 2015-07-07 17:02 Damian, Alexandru
2015-07-09 11:34 ` Barros Pena, Belen
0 siblings, 1 reply; 16+ messages in thread
From: Damian, Alexandru @ 2015-07-07 17:02 UTC (permalink / raw)
To: toaster@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 269 bytes --]
There are two new patches for review.
- fixing the object updates on build requests that failed to start
- bug fixing for #7945 - loadconf excepts when called on file that does not
live in git repo
Cheers,
Alex
--
Alex Damian
Yocto Project
SSG / OTC
[-- Attachment #2: Type: text/html, Size: 950 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-07 16:27 ` Damian, Alexandru
@ 2015-07-08 14:52 ` Barros Pena, Belen
2015-07-13 16:30 ` Michael Wood
0 siblings, 1 reply; 16+ messages in thread
From: Barros Pena, Belen @ 2015-07-08 14:52 UTC (permalink / raw)
To: Damian, Alexandru; +Cc: toaster@yoctoproject.org
On 07/07/2015 17:27, "Damian, Alexandru" <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> wrote:
>
>
>
>On 07/07/2015 15:10, "Damian, Alexandru" <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
>
>
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-07 17:02 Damian, Alexandru
@ 2015-07-09 11:34 ` Barros Pena, Belen
0 siblings, 0 replies; 16+ messages in thread
From: Barros Pena, Belen @ 2015-07-09 11:34 UTC (permalink / raw)
To: Damian, Alexandru, toaster@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 2443 bytes --]
Thanks, Alex.
I have a couple of comments (inline).
On 07/07/2015 18:02, "Damian, Alexandru" <alexandru.damian@intel.com>
wrote:
>There are two new patches for review.
>
>
>- fixing the object updates on build requests that failed to start
Toaster now starts with a db that has failed build requests in it, and I
can see the information :)
I've attached a screenshot of what I see and it's a bit strange though.
First, I see 1 error and 1 exception, but only the exception has content
(the error is empty). I am not sure if this is just because the data was
logged before the changes you've made, or if there is something funny
going on.
The second issue I have is presenting things like "Bitbake server did not
start in 5 seconds" as Toaster exceptions, since this is what caused the
whole thing to fail in the first place. I know that strictly this is
probably a Toaster exception, but I am not sure this sends the right
message to users. So what about a simple rule? If the build fails before
starting proper (at the build request stage), any Toaster exceptions are
presented as errors. If the build succeeds or fails past the build request
stage, we show Toaster exceptions as Toaster exceptions.
Would that we possible?
>- bug fixing for #7945 - loadconf excepts when called on file that does
>not live in git repo
The error I am seeing says:
Failure while trying to import the toaster config file: Error while
looking for remote "origin" in ""
Although I think I follow (because I know what the problem is), I am not
sure whether someone who doesn't know what's going on will get it. Could
we maybe say:
* "Toaster failed to import the selected configuration file" (this is just
to make the sentence shorter, to avoid abbreviations, and match the
language we use in the rest of the set up process)
* Then have two versions of this message: one when there is a value for
the Git URI, and another one when the Git URI is blank (like in this
case). The first one could stay as is, the second would say something
like: "Toaster did not find a Git repository. If you are using a Yocto
Project release tarball, please clone a Yocto Project repository instead."
The above is just a suggestion. If anybody can think of something better,
please say so
Thanks!
Belén
>
>
>
>
>Cheers,
>Alex
>
>
>--
>Alex Damian
>Yocto Project
>
>SSG / OTC
>
>
>
[-- Attachment #2: failed-build-request.png --]
[-- Type: image/png, Size: 194603 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-08 14:52 ` Barros Pena, Belen
@ 2015-07-13 16:30 ` Michael Wood
2015-07-15 12:31 ` Damian, Alexandru
0 siblings, 1 reply; 16+ messages in thread
From: Michael Wood @ 2015-07-13 16:30 UTC (permalink / raw)
To: toaster
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>
> 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> wrote:
>>
>>
>>
>> On 07/07/2015 15:10, "Damian, Alexandru" <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
>>
>>
>>
>>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-13 16:30 ` Michael Wood
@ 2015-07-15 12:31 ` Damian, Alexandru
2015-07-17 10:27 ` Michael Wood
0 siblings, 1 reply; 16+ messages in thread
From: Damian, Alexandru @ 2015-07-15 12:31 UTC (permalink / raw)
To: Michael Wood; +Cc: toaster@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 3999 bytes --]
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>
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>
>> 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> wrote:
>>>
>>>
>>>
>>> On 07/07/2015 15:10, "Damian, Alexandru" <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
> https://lists.yoctoproject.org/listinfo/toaster
>
--
Alex Damian
Yocto Project
SSG / OTC
[-- Attachment #2: Type: text/html, Size: 6476 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-15 12:31 ` Damian, Alexandru
@ 2015-07-17 10:27 ` Michael Wood
2015-07-21 11:26 ` Damian, Alexandru
0 siblings, 1 reply; 16+ messages in thread
From: Michael Wood @ 2015-07-17 10:27 UTC (permalink / raw)
To: Damian, Alexandru; +Cc: toaster@yoctoproject.org
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.
>
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-17 10:27 ` Michael Wood
@ 2015-07-21 11:26 ` Damian, Alexandru
2015-07-21 12:39 ` Michael Wood
0 siblings, 1 reply; 16+ messages in thread
From: Damian, Alexandru @ 2015-07-21 11:26 UTC (permalink / raw)
To: Michael Wood; +Cc: toaster@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 8625 bytes --]
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.
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@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@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.
>>
>>
> ---------------------------------------------------------------------
> 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
[-- Attachment #2: Type: text/html, Size: 13540 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-21 11:26 ` Damian, Alexandru
@ 2015-07-21 12:39 ` Michael Wood
2015-07-21 12:44 ` Damian, Alexandru
0 siblings, 1 reply; 16+ messages in thread
From: Michael Wood @ 2015-07-21 12:39 UTC (permalink / raw)
To: Damian, Alexandru; +Cc: toaster@yoctoproject.org
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 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@intel.com <mailto:michael.g.wood@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@intel.com <mailto:michael.g.wood@intel.com>
> <mailto: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>
> <mailto: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>
> <mailto: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>
> <mailto: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>
> <mailto: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.
>
>
> ---------------------------------------------------------------------
> 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.
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-21 12:39 ` Michael Wood
@ 2015-07-21 12:44 ` Damian, Alexandru
2015-07-22 14:44 ` Damian, Alexandru
0 siblings, 1 reply; 16+ messages in thread
From: Damian, Alexandru @ 2015-07-21 12:44 UTC (permalink / raw)
To: Michael Wood; +Cc: toaster@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 12168 bytes --]
On Tue, Jul 21, 2015 at 1:39 PM, Michael Wood <michael.g.wood@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@intel.com
>> <mailto:michael.g.wood@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@intel.com <mailto:michael.g.wood@intel.com>
>> <mailto: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>
>> <mailto: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>
>> <mailto: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>
>> <mailto: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>
>> <mailto: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.
>>
>>
>> ---------------------------------------------------------------------
>> 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
[-- Attachment #2: Type: text/html, Size: 17285 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-21 12:44 ` Damian, Alexandru
@ 2015-07-22 14:44 ` Damian, Alexandru
2015-07-23 13:02 ` Damian, Alexandru
0 siblings, 1 reply; 16+ messages in thread
From: Damian, Alexandru @ 2015-07-22 14:44 UTC (permalink / raw)
To: Michael Wood; +Cc: toaster@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 12814 bytes --]
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@intel.com> wrote:
>
>
> On Tue, Jul 21, 2015 at 1:39 PM, Michael Wood <michael.g.wood@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@intel.com
>>> <mailto:michael.g.wood@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@intel.com <mailto:michael.g.wood@intel.com>
>>> <mailto: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>
>>> <mailto: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>
>>> <mailto: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>
>>> <mailto: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>
>>> <mailto: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.
>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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
[-- Attachment #2: Type: text/html, Size: 18088 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-22 14:44 ` Damian, Alexandru
@ 2015-07-23 13:02 ` Damian, Alexandru
2015-07-24 15:06 ` Damian, Alexandru
0 siblings, 1 reply; 16+ messages in thread
From: Damian, Alexandru @ 2015-07-23 13:02 UTC (permalink / raw)
To: Michael Wood; +Cc: toaster@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 13639 bytes --]
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@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@intel.com> wrote:
>
>>
>>
>> On Tue, Jul 21, 2015 at 1:39 PM, Michael Wood <michael.g.wood@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@intel.com <mailto:michael.g.wood@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@intel.com <mailto:michael.g.wood@intel.com>
>>>> <mailto: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>
>>>> <mailto: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>
>>>> <mailto: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>
>>>> <mailto: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>
>>>> <mailto: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.
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------------
>>>> 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
[-- Attachment #2: Type: text/html, Size: 19432 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-23 13:02 ` Damian, Alexandru
@ 2015-07-24 15:06 ` Damian, Alexandru
2015-07-24 15:11 ` Damian, Alexandru
0 siblings, 1 reply; 16+ messages in thread
From: Damian, Alexandru @ 2015-07-24 15:06 UTC (permalink / raw)
To: Michael Wood; +Cc: toaster@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 14610 bytes --]
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@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@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@intel.com> wrote:
>>
>>>
>>>
>>> On Tue, Jul 21, 2015 at 1:39 PM, Michael Wood <michael.g.wood@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@intel.com <mailto:michael.g.wood@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@intel.com <mailto:michael.g.wood@intel.com>
>>>>> <mailto: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>
>>>>> <mailto: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>
>>>>> <mailto: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>
>>>>> <mailto: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>
>>>>> <mailto: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.
>>>>>
>>>>>
>>>>>
>>>>> ---------------------------------------------------------------------
>>>>> 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
[-- Attachment #2: Type: text/html, Size: 21278 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [review-request] adamian/20150707_bugs
2015-07-24 15:06 ` Damian, Alexandru
@ 2015-07-24 15:11 ` Damian, Alexandru
0 siblings, 0 replies; 16+ messages in thread
From: Damian, Alexandru @ 2015-07-24 15:11 UTC (permalink / raw)
To: Michael Wood; +Cc: toaster@yoctoproject.org
[-- Attachment #1: Type: text/plain, Size: 15405 bytes --]
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@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@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@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@intel.com> wrote:
>>>
>>>>
>>>>
>>>> On Tue, Jul 21, 2015 at 1:39 PM, Michael Wood <michael.g.wood@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@intel.com <mailto:michael.g.wood@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@intel.com <mailto:michael.g.wood@intel.com>
>>>>>> <mailto: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>
>>>>>> <mailto: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>
>>>>>> <mailto: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>
>>>>>> <mailto: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>
>>>>>> <mailto: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.
>>>>>>
>>>>>>
>>>>>>
>>>>>> ---------------------------------------------------------------------
>>>>>> 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
[-- Attachment #2: Type: text/html, Size: 22623 bytes --]
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-07-24 15:11 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-07-07 14:10 [review-request] adamian/20150707_bugs Damian, Alexandru
2015-07-07 14:54 ` Barros Pena, Belen
2015-07-07 16:27 ` Damian, Alexandru
2015-07-08 14:52 ` Barros Pena, Belen
2015-07-13 16:30 ` Michael Wood
2015-07-15 12:31 ` Damian, Alexandru
2015-07-17 10:27 ` Michael Wood
2015-07-21 11:26 ` Damian, Alexandru
2015-07-21 12:39 ` Michael Wood
2015-07-21 12:44 ` Damian, Alexandru
2015-07-22 14:44 ` Damian, Alexandru
2015-07-23 13:02 ` Damian, Alexandru
2015-07-24 15:06 ` Damian, Alexandru
2015-07-24 15:11 ` Damian, Alexandru
-- strict thread matches above, loose matches on Subject: below --
2015-07-07 17:02 Damian, Alexandru
2015-07-09 11:34 ` Barros Pena, Belen
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.