All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: "Damian, Alexandru" <alexandru.damian@intel.com>
To: Michael Wood <michael.g.wood@intel.com>
Cc: "toaster@yoctoproject.org" <toaster@yoctoproject.org>
Subject: Re: [review-request] ed/toaster/misc
Date: Tue, 14 Jul 2015 12:18:02 +0100	[thread overview]
Message-ID: <CAJ2CSBsbh7rZiLkY5xg5xU5AfcuOrefvTq7CSUSqCyJQ+5Vi9w@mail.gmail.com> (raw)
In-Reply-To: <55A3EBB9.5060109@intel.com>

[-- Attachment #1: Type: text/plain, Size: 3282 bytes --]

Hi,

I have some comments below:

*toaster: get rid of _createdirpath function*

    os.mkdirs is missing the mode specification; I am not sure it matters,
though.


*toaster: Wait for toaster gui to come*

  I am not sure which logger should to be used here; the reason for
changing the logger in the first place (which was originally defined as
"BitBake" was that the "BitBake" logger settings are modified in
bitbake/lib/bb/__init__.py, and it is targeted at the inner Bitbake core;
some updates there changed the original output, with output disappearing
from the console being an obvious effect.
Messing with "BitBake" logger settings with  this may have side effects
throught the bitbake code, so I refrained from touching that

  I considered using the "toaster" logger that is defined in
bitbake/lib/toaster/toastermain/settings.py and outputs strictly to
console; I felt likewise reluctant to use this logger because changing it
to output to a file would be affect the way the django application is
logging.

  ToasterLogger is not defined anywhere, and this is why I used it here. I
am not sure I understand the root cause of why the code breaks when using
"ToasterLogger" instead of "BitBake"; can you please assess the root cause
and get a solution to have messages properly logged to the toaster_ui.log
file using a logger that's not BitBake ?


Otherwise, all fine.

Thank you,
Alex



On Mon, Jul 13, 2015 at 5:47 PM, Michael Wood <michael.g.wood@intel.com>
wrote:

> On 10/07/15 15:48, Ed Bartosh wrote:
>
>> Hi reviewers,
>>
>> Please review fix for YOCTO: #7965: Wait for toaster gui to come
>> and couple of other changes:
>> 1. tiny refactoring change:  get rid of _createdirpath function
>> 2. and small fix: Fix usage of wrong variables
>>
>> --
>> Regards,
>> Ed
>>
>
> * 7cee08d toaster: get rid of _createdirpath function
> * eae47f7 toaster: Fix usage of wrong variables
> * 321d6ae toaster: Wait for toaster gui to come
>
> Looks fine to me, though not familiar enough with build stuff to fully OK
> this branch.
>
> While you're in localhostbecontroller
>
> We could also get rid of
>
>     def _shellcmd(self, command, cwd = None):
>         if cwd is None:
>             cwd = self.be.sourcedir
>
>         #logger.debug("lbc_shellcmmd: (%s) %s" % (cwd, command))
>         p = subprocess.Popen(command, cwd = cwd, shell=True,
> stdout=subprocess.PIPE, stderr=subprocess.PIPE)
>         (out,err) = p.communicate()
>         p.wait()
>         if p.returncode:
>             if len(err) == 0:
>                 err = "command: %s \n%s" % (command, out)
>             else:
>                 err = "command: %s \n%s" % (command, err)
>             #logger.warn("localhostbecontroller: shellcmd error %s" % err)
>             raise ShellCmdException(err)
>         else:
>             #logger.debug("localhostbecontroller: shellcmd success")
>             return out
>
>
> I believe with:
>
> subprocess.check_output
>
> Thanks,
>
> Michael
>
> --
> _______________________________________________
> toaster mailing list
> toaster@yoctoproject.org
> https://lists.yoctoproject.org/listinfo/toaster
>



-- 
Alex Damian
Yocto Project
SSG / OTC

[-- Attachment #2: Type: text/html, Size: 5157 bytes --]

  reply	other threads:[~2015-07-14 11:18 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-10 14:48 [review-request] ed/toaster/misc Ed Bartosh
2015-07-13 16:47 ` Michael Wood
2015-07-14 11:18   ` Damian, Alexandru [this message]
2015-07-14 13:37     ` Ed Bartosh
2015-07-19 19:35       ` Ed Bartosh
2015-07-14 13:06   ` Ed Bartosh
2015-07-14 13:37     ` Damian, Alexandru
2015-07-14 13:52       ` Ed Bartosh
2015-07-24 16:22         ` Ed Bartosh
2015-07-27 12:26           ` Damian, Alexandru
2015-07-28 12:35             ` Ed Bartosh
2015-07-28 14:00               ` Damian, Alexandru

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAJ2CSBsbh7rZiLkY5xg5xU5AfcuOrefvTq7CSUSqCyJQ+5Vi9w@mail.gmail.com \
    --to=alexandru.damian@intel.com \
    --cc=michael.g.wood@intel.com \
    --cc=toaster@yoctoproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.