* [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
* 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-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
* [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 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
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.
![[-- Attachment #2: failed-build-request.png --] [-- Type: image/png, Size: 194603 bytes --]](../../D1C413E6.5F580%25belen.barros.pena@intel.com/2-failed-build-request.png){kind=link}