All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] s3.py: Add support for fetching source mirrors/minor cleanup
@ 2017-03-28 11:39 Elizabeth 'pidge' Flanagan
  2017-03-28 12:56 ` Richard Purdie
  0 siblings, 1 reply; 5+ messages in thread
From: Elizabeth 'pidge' Flanagan @ 2017-03-28 11:39 UTC (permalink / raw)
  To: bitbake-devel

This commits main purpose is to add support for fetching source
mirrors. In the current incarnation:

SOURCE_MIRROR_URL ?= "s3://mybucket/downloads"

will fail for two reasons. First, download doesn't support it,
but second, without aws included in HOSTTOOLS you'll end up
with aws not being found by bitbake (for either source mirrors or
sstate mirrors).

Part of this is fixed with this commit. However, this will still
fail if HOSTTOOLS doesn't include 'aws' in bitbake.conf. I've another
commit or two to fix that as well.

I've also DRYed up some of the error handling, removed the hardcoded
aws and added some logging.

Signed-off-by: Elizabeth 'pidge' Flanagan <pidge@toganlabs.com>
---
 lib/bb/fetch2/s3.py | 20 ++++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/lib/bb/fetch2/s3.py b/lib/bb/fetch2/s3.py
index 27993aa..dbd86f4 100644
--- a/lib/bb/fetch2/s3.py
+++ b/lib/bb/fetch2/s3.py
@@ -34,6 +34,7 @@ import urllib.request, urllib.parse, urllib.error
 from bb.fetch2 import FetchMethod
 from bb.fetch2 import FetchError
 from bb.fetch2 import runfetchcmd
+from bb.fetch2 import logger
 
 class S3(FetchMethod):
     """Class to fetch urls via 'aws s3'"""
@@ -60,8 +61,13 @@ class S3(FetchMethod):
         Fetch urls
         Assumes localpath was called first
         """
-
-        cmd = 'aws s3 cp s3://%s%s %s' % (ud.host, ud.path, ud.localpath)
+        if 'downloadfilename' in ud.parm:
+            dldir = d.getVar("DL_DIR", True)
+            bb.utils.mkdirhier(os.path.dirname(dldir + os.sep + ud.localfile))
+            cmd = '%s cp s3://%s%s %s%s%s' % (d.getVar("FETCHCMD_s3"), ud.host, ud.path, dldir, os.sep, ud.localpath)
+        else:
+            cmd = '%s cp s3://%s%s %s' % (d.getVar("FETCHCMD_s3"), ud.host, ud.path, ud.localpath)
+        logger.debug(2, "Fetching %s using command '%s'" % (ud.url, cmd))
         bb.fetch2.check_network_access(d, cmd, ud.url)
         runfetchcmd(cmd, d)
 
@@ -70,11 +76,11 @@ class S3(FetchMethod):
         # tool with a little healthy suspicion).
 
         if not os.path.exists(ud.localpath):
-            raise FetchError("The aws cp command returned success for s3://%s%s but %s doesn't exist?!" % (ud.host, ud.path, ud.localpath))
+            raise FetchError("The command  %s returned success but %s doesn't exist?!" % (cmd, ud.localpath))
 
         if os.path.getsize(ud.localpath) == 0:
             os.remove(ud.localpath)
-            raise FetchError("The aws cp command for s3://%s%s resulted in a zero size file?! Deleting and failing since this isn't right." % (ud.host, ud.path))
+            raise FetchError("The command %s resulted in a zero size file?! Deleting and failing since this isn't right." % (cmd))
 
         return True
 
@@ -83,7 +89,9 @@ class S3(FetchMethod):
         Check the status of a URL
         """
 
-        cmd = 'aws s3 ls s3://%s%s' % (ud.host, ud.path)
+        cmd = '%s ls s3://%s%s' % (d.getVar("FETCHCMD_s3"), ud.host, ud.path)
+        logger.debug(2, "Checking %s using command '%s'" % (ud.url, cmd))
+
         bb.fetch2.check_network_access(d, cmd, ud.url)
         output = runfetchcmd(cmd, d)
 
@@ -91,6 +99,6 @@ class S3(FetchMethod):
         # is not found, so check output of the command to confirm success.
 
         if not output:
-            raise FetchError("The aws ls command for s3://%s%s gave empty output" % (ud.host, ud.path))
+            raise FetchError("The command %s gave empty output" % (cmd))
 
         return True
-- 
1.9.1



^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] s3.py: Add support for fetching source mirrors/minor cleanup
  2017-03-28 11:39 [PATCH] s3.py: Add support for fetching source mirrors/minor cleanup Elizabeth 'pidge' Flanagan
@ 2017-03-28 12:56 ` Richard Purdie
  2017-03-28 13:05   ` Beth 'pidge' Flanagan
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Purdie @ 2017-03-28 12:56 UTC (permalink / raw)
  To: Elizabeth 'pidge' Flanagan, bitbake-devel

On Tue, 2017-03-28 at 13:39 +0200, Elizabeth 'pidge' Flanagan wrote:
> This commits main purpose is to add support for fetching source
> mirrors. In the current incarnation:
> 
> SOURCE_MIRROR_URL ?= "s3://mybucket/downloads"
> 
> will fail for two reasons. First, download doesn't support it,
> but second, without aws included in HOSTTOOLS you'll end up
> with aws not being found by bitbake (for either source mirrors or
> sstate mirrors).
> 
> Part of this is fixed with this commit. However, this will still
> fail if HOSTTOOLS doesn't include 'aws' in bitbake.conf. I've another
> commit or two to fix that as well.
> 
> I've also DRYed up some of the error handling, removed the hardcoded
> aws and added some logging.
> 
> Signed-off-by: Elizabeth 'pidge' Flanagan <pidge@toganlabs.com>
> ---
>  lib/bb/fetch2/s3.py | 20 ++++++++++++++------
>  1 file changed, 14 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/bb/fetch2/s3.py b/lib/bb/fetch2/s3.py
> index 27993aa..dbd86f4 100644
> --- a/lib/bb/fetch2/s3.py
> +++ b/lib/bb/fetch2/s3.py
> @@ -34,6 +34,7 @@ import urllib.request, urllib.parse, urllib.error
>  from bb.fetch2 import FetchMethod
>  from bb.fetch2 import FetchError
>  from bb.fetch2 import runfetchcmd
> +from bb.fetch2 import logger
>  
>  class S3(FetchMethod):
>      """Class to fetch urls via 'aws s3'"""
> @@ -60,8 +61,13 @@ class S3(FetchMethod):
>          Fetch urls
>          Assumes localpath was called first
>          """
> -
> -        cmd = 'aws s3 cp s3://%s%s %s' % (ud.host, ud.path,
> ud.localpath)
> +        if 'downloadfilename' in ud.parm:
> +            dldir = d.getVar("DL_DIR", True)
> +            bb.utils.mkdirhier(os.path.dirname(dldir + os.sep +
> ud.localfile))
> +            cmd = '%s cp s3://%s%s %s%s%s' %
> (d.getVar("FETCHCMD_s3"), ud.host, ud.path, dldir, os.sep,
> ud.localpath)
> +        else:
> +            cmd = '%s cp s3://%s%s %s' % (d.getVar("FETCHCMD_s3"),
> ud.host, ud.path, ud.localpath)
> +        logger.debug(2, "Fetching %s using command '%s'" % (ud.url,
> cmd))

Do you actually need/use FETCHCMD_s3 ?

I'd prefer not to add this unless we really do need it.

Cheers,

Richard


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] s3.py: Add support for fetching source mirrors/minor cleanup
  2017-03-28 12:56 ` Richard Purdie
@ 2017-03-28 13:05   ` Beth 'pidge' Flanagan
  2017-03-28 13:21     ` Richard Purdie
  0 siblings, 1 reply; 5+ messages in thread
From: Beth 'pidge' Flanagan @ 2017-03-28 13:05 UTC (permalink / raw)
  To: Richard Purdie, bitbake-devel

On Tue, 2017-03-28 at 13:56 +0100, Richard Purdie wrote:
> On Tue, 2017-03-28 at 13:39 +0200, Elizabeth 'pidge' Flanagan wrote:
> > 
> > This commits main purpose is to add support for fetching source
> > mirrors. In the current incarnation:
> > 
> > SOURCE_MIRROR_URL ?= "s3://mybucket/downloads"
> > 
> > will fail for two reasons. First, download doesn't support it,
> > but second, without aws included in HOSTTOOLS you'll end up
> > with aws not being found by bitbake (for either source mirrors or
> > sstate mirrors).
> > 
> > Part of this is fixed with this commit. However, this will still
> > fail if HOSTTOOLS doesn't include 'aws' in bitbake.conf. I've
> > another
> > commit or two to fix that as well.
> > 
> > I've also DRYed up some of the error handling, removed the
> > hardcoded
> > aws and added some logging.
> > 
> > Signed-off-by: Elizabeth 'pidge' Flanagan <pidge@toganlabs.com>
> > ---
> >  lib/bb/fetch2/s3.py | 20 ++++++++++++++------
> >  1 file changed, 14 insertions(+), 6 deletions(-)
> > 
> > diff --git a/lib/bb/fetch2/s3.py b/lib/bb/fetch2/s3.py
> > index 27993aa..dbd86f4 100644
> > --- a/lib/bb/fetch2/s3.py
> > +++ b/lib/bb/fetch2/s3.py
> > @@ -34,6 +34,7 @@ import urllib.request, urllib.parse, urllib.error
> >  from bb.fetch2 import FetchMethod
> >  from bb.fetch2 import FetchError
> >  from bb.fetch2 import runfetchcmd
> > +from bb.fetch2 import logger
> >  
> >  class S3(FetchMethod):
> >      """Class to fetch urls via 'aws s3'"""
> > @@ -60,8 +61,13 @@ class S3(FetchMethod):
> >          Fetch urls
> >          Assumes localpath was called first
> >          """
> > -
> > -        cmd = 'aws s3 cp s3://%s%s %s' % (ud.host, ud.path,
> > ud.localpath)
> > +        if 'downloadfilename' in ud.parm:
> > +            dldir = d.getVar("DL_DIR", True)
> > +            bb.utils.mkdirhier(os.path.dirname(dldir + os.sep +
> > ud.localfile))
> > +            cmd = '%s cp s3://%s%s %s%s%s' %
> > (d.getVar("FETCHCMD_s3"), ud.host, ud.path, dldir, os.sep,
> > ud.localpath)
> > +        else:
> > +            cmd = '%s cp s3://%s%s %s' % (d.getVar("FETCHCMD_s3"),
> > ud.host, ud.path, ud.localpath)
> > +        logger.debug(2, "Fetching %s using command '%s'" %
> > (ud.url,
> > cmd))
> Do you actually need/use FETCHCMD_s3 ?
> 

I use it so I can rip out aws-cli and use s3cmd.

-b

> I'd prefer not to add this unless we really do need it.
> 
> Cheers,
> 
> Richard




^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] s3.py: Add support for fetching source mirrors/minor cleanup
  2017-03-28 13:05   ` Beth 'pidge' Flanagan
@ 2017-03-28 13:21     ` Richard Purdie
  2017-03-28 13:31       ` Beth 'pidge' Flanagan
  0 siblings, 1 reply; 5+ messages in thread
From: Richard Purdie @ 2017-03-28 13:21 UTC (permalink / raw)
  To: pidge, bitbake-devel

On Tue, 2017-03-28 at 15:05 +0200, Beth 'pidge' Flanagan wrote:
> On Tue, 2017-03-28 at 13:56 +0100, Richard Purdie wrote:
> > 
> > On Tue, 2017-03-28 at 13:39 +0200, Elizabeth 'pidge' Flanagan
> > wrote:
> > > 
> > > 
> > > This commits main purpose is to add support for fetching source
> > > mirrors. In the current incarnation:
> > > 
> > > SOURCE_MIRROR_URL ?= "s3://mybucket/downloads"
> > > 
> > > will fail for two reasons. First, download doesn't support it,
> > > but second, without aws included in HOSTTOOLS you'll end up
> > > with aws not being found by bitbake (for either source mirrors or
> > > sstate mirrors).
> > > 
> > > Part of this is fixed with this commit. However, this will still
> > > fail if HOSTTOOLS doesn't include 'aws' in bitbake.conf. I've
> > > another
> > > commit or two to fix that as well.
> > > 
> > > I've also DRYed up some of the error handling, removed the
> > > hardcoded
> > > aws and added some logging.
> > > 
> > > Signed-off-by: Elizabeth 'pidge' Flanagan <pidge@toganlabs.com>
> > > ---
> > >  lib/bb/fetch2/s3.py | 20 ++++++++++++++------
> > >  1 file changed, 14 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/lib/bb/fetch2/s3.py b/lib/bb/fetch2/s3.py
> > > index 27993aa..dbd86f4 100644
> > > --- a/lib/bb/fetch2/s3.py
> > > +++ b/lib/bb/fetch2/s3.py
> > > @@ -34,6 +34,7 @@ import urllib.request, urllib.parse,
> > > urllib.error
> > >  from bb.fetch2 import FetchMethod
> > >  from bb.fetch2 import FetchError
> > >  from bb.fetch2 import runfetchcmd
> > > +from bb.fetch2 import logger
> > >  
> > >  class S3(FetchMethod):
> > >      """Class to fetch urls via 'aws s3'"""
> > > @@ -60,8 +61,13 @@ class S3(FetchMethod):
> > >          Fetch urls
> > >          Assumes localpath was called first
> > >          """
> > > -
> > > -        cmd = 'aws s3 cp s3://%s%s %s' % (ud.host, ud.path,
> > > ud.localpath)
> > > +        if 'downloadfilename' in ud.parm:
> > > +            dldir = d.getVar("DL_DIR", True)
> > > +            bb.utils.mkdirhier(os.path.dirname(dldir + os.sep +
> > > ud.localfile))
> > > +            cmd = '%s cp s3://%s%s %s%s%s' %
> > > (d.getVar("FETCHCMD_s3"), ud.host, ud.path, dldir, os.sep,
> > > ud.localpath)
> > > +        else:
> > > +            cmd = '%s cp s3://%s%s %s' %
> > > (d.getVar("FETCHCMD_s3"),
> > > ud.host, ud.path, ud.localpath)
> > > +        logger.debug(2, "Fetching %s using command '%s'" %
> > > (ud.url,
> > > cmd))
> > Do you actually need/use FETCHCMD_s3 ?
> > 
> I use it so I can rip out aws-cli and use s3cmd.

Ok. The way this needs to work then is that in __init__, you set a
default command and then if d.getVar("FETCHCMD_s3") is set, overwrite
it. This is so that the fetcher works without having to FETCHCMD_s3 (in
common with all the other fetchers. You'd then use ud.cmd instead of
all the getVars.

Cheers,

Richard


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] s3.py: Add support for fetching source mirrors/minor cleanup
  2017-03-28 13:21     ` Richard Purdie
@ 2017-03-28 13:31       ` Beth 'pidge' Flanagan
  0 siblings, 0 replies; 5+ messages in thread
From: Beth 'pidge' Flanagan @ 2017-03-28 13:31 UTC (permalink / raw)
  To: Richard Purdie, bitbake-devel

On Tue, 2017-03-28 at 14:21 +0100, Richard Purdie wrote:
> On Tue, 2017-03-28 at 15:05 +0200, Beth 'pidge' Flanagan wrote:
> > 
> > On Tue, 2017-03-28 at 13:56 +0100, Richard Purdie wrote:
> > > 
> > > 
> > > On Tue, 2017-03-28 at 13:39 +0200, Elizabeth 'pidge' Flanagan
> > > wrote:
> > > > 
> > > > 
> > > > 
> > > > This commits main purpose is to add support for fetching source
> > > > mirrors. In the current incarnation:
> > > > 
> > > > SOURCE_MIRROR_URL ?= "s3://mybucket/downloads"
> > > > 
> > > > will fail for two reasons. First, download doesn't support it,
> > > > but second, without aws included in HOSTTOOLS you'll end up
> > > > with aws not being found by bitbake (for either source mirrors
> > > > or
> > > > sstate mirrors).
> > > > 
> > > > Part of this is fixed with this commit. However, this will
> > > > still
> > > > fail if HOSTTOOLS doesn't include 'aws' in bitbake.conf. I've
> > > > another
> > > > commit or two to fix that as well.
> > > > 
> > > > I've also DRYed up some of the error handling, removed the
> > > > hardcoded
> > > > aws and added some logging.
> > > > 
> > > > Signed-off-by: Elizabeth 'pidge' Flanagan <pidge@toganlabs.com>
> > > > ---
> > > >  lib/bb/fetch2/s3.py | 20 ++++++++++++++------
> > > >  1 file changed, 14 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/lib/bb/fetch2/s3.py b/lib/bb/fetch2/s3.py
> > > > index 27993aa..dbd86f4 100644
> > > > --- a/lib/bb/fetch2/s3.py
> > > > +++ b/lib/bb/fetch2/s3.py
> > > > @@ -34,6 +34,7 @@ import urllib.request, urllib.parse,
> > > > urllib.error
> > > >  from bb.fetch2 import FetchMethod
> > > >  from bb.fetch2 import FetchError
> > > >  from bb.fetch2 import runfetchcmd
> > > > +from bb.fetch2 import logger
> > > >  
> > > >  class S3(FetchMethod):
> > > >      """Class to fetch urls via 'aws s3'"""
> > > > @@ -60,8 +61,13 @@ class S3(FetchMethod):
> > > >          Fetch urls
> > > >          Assumes localpath was called first
> > > >          """
> > > > -
> > > > -        cmd = 'aws s3 cp s3://%s%s %s' % (ud.host, ud.path,
> > > > ud.localpath)
> > > > +        if 'downloadfilename' in ud.parm:
> > > > +            dldir = d.getVar("DL_DIR", True)
> > > > +            bb.utils.mkdirhier(os.path.dirname(dldir + os.sep
> > > > +
> > > > ud.localfile))
> > > > +            cmd = '%s cp s3://%s%s %s%s%s' %
> > > > (d.getVar("FETCHCMD_s3"), ud.host, ud.path, dldir, os.sep,
> > > > ud.localpath)
> > > > +        else:
> > > > +            cmd = '%s cp s3://%s%s %s' %
> > > > (d.getVar("FETCHCMD_s3"),
> > > > ud.host, ud.path, ud.localpath)
> > > > +        logger.debug(2, "Fetching %s using command '%s'" %
> > > > (ud.url,
> > > > cmd))
> > > Do you actually need/use FETCHCMD_s3 ?
> > > 
> > I use it so I can rip out aws-cli and use s3cmd.
> Ok. The way this needs to work then is that in __init__, you set a
> default command and then if d.getVar("FETCHCMD_s3") is set, overwrite
> it. This is so that the fetcher works without having to FETCHCMD_s3
> (in
> common with all the other fetchers. You'd then use ud.cmd instead of
> all the getVars.
> 

Ah, ok. v2 incoming in a bit.

> Cheers,
> Richard


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-03-28 13:31 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-28 11:39 [PATCH] s3.py: Add support for fetching source mirrors/minor cleanup Elizabeth 'pidge' Flanagan
2017-03-28 12:56 ` Richard Purdie
2017-03-28 13:05   ` Beth 'pidge' Flanagan
2017-03-28 13:21     ` Richard Purdie
2017-03-28 13:31       ` Beth 'pidge' Flanagan

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.