Merge lp:~gary/charms/precise/juju-gui/run-without-ppa into lp:~juju-gui/charms/precise/juju-gui/trunk

Proposed by Gary Poster
Status: Merged
Merged at revision: 22
Proposed branch: lp:~gary/charms/precise/juju-gui/run-without-ppa
Merge into: lp:~juju-gui/charms/precise/juju-gui/trunk
Diff against target: 0 lines
To merge this branch: bzr merge lp:~gary/charms/precise/juju-gui/run-without-ppa
Reviewer Review Type Date Requested Status
Juju GUI Hackers Pending
Review via email: mp+142568@code.launchpad.net

Description of the change

Make charm work without environment Juju PPA

As discovered by therve and myself, the charm does not work if "juju-origin: ppa" is not in your environments.yaml for the given Juju environment. These changes work around the problem.

https://codereview.appspot.com/7073051/

To post a comment you must log in.
Revision history for this message
Gary Poster (gary) wrote :
Download full text (4.3 KiB)

Reviewers: mp+142568_code.launchpad.net,

Message:
Please take a look.

Description:
Make charm work without environment Juju PPA

As discovered by therve and myself, the charm does not work if
"juju-origin: ppa" is not in your environments.yaml for the given Juju
environment. These changes work around the problem.

https://code.launchpad.net/~gary/charms/precise/juju-gui/run-without-ppa/+merge/142568

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7073051/

Affected files:
   A [revision details]
   A hooks/bootstrap_utils.py
   M hooks/install
   M revision

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: <email address hidden>
+New revision: <email address hidden>

Index: revision
=== modified file 'revision'
--- revision 2013-01-03 20:16:28 +0000
+++ revision 2013-01-09 18:12:09 +0000
@@ -1,1 +1,1 @@
-18
+19

Index: hooks/bootstrap_utils.py
=== added file 'hooks/bootstrap_utils.py'
--- hooks/bootstrap_utils.py 1970-01-01 00:00:00 +0000
+++ hooks/bootstrap_utils.py 2013-01-09 18:12:09 +0000
@@ -0,0 +1,54 @@
+# These are actually maintained in python-shelltoolbox. Precise does not
have
+# that package, so we need to bootstrap the process by copying the
functions
+# we need here.
+
+import subprocess
+
+try:
+ import shelltoolbox
+except ImportError:
+ def run(*args, **kwargs):
+ """Run the command with the given arguments.
+
+ The first argument is the path to the command to run.
+ Subsequent arguments are command-line arguments to be passed.
+
+ This function accepts all optional keyword arguments accepted by
+ `subprocess.Popen`.
+ """
+ args = [i for i in args if i is not None]
+ pipe = subprocess.PIPE
+ process = subprocess.Popen(
+ args, stdout=kwargs.pop('stdout', pipe),
+ stderr=kwargs.pop('stderr', pipe),
+ close_fds=kwargs.pop('close_fds', True), **kwargs)
+ stdout, stderr = process.communicate()
+ if process.returncode:
+ exception = subprocess.CalledProcessError(
+ process.returncode, repr(args))
+ # The output argument of `CalledProcessError` was introduced
in Python
+ # 2.7. Monkey patch the output here to avoid TypeErrors in
older
+ # versions of Python, still preserving the output in Python
2.7.
+ exception.output = ''.join(filter(None, [stdout, stderr]))
+ raise exception
+ return stdout
+
+ def install_extra_repositories(*repositories):
+ """Install all of the extra repositories and update apt.
+
+ Given repositories can contain a "{distribution}" placeholder,
that will
+ be replaced by current distribution codename.
+
+ :raises: subprocess.CalledProcessError
+ """
+ distribution = run('lsb_release', '-cs').strip()
+ # Starting from Oneiric, `apt-add-repository` is interactive by
+ # ...

Read more...

Revision history for this message
Madison Scott-Clary (makyo) wrote :

LGTM, thanks for the work-around. No changes here, but I do have a
question: will this eventually not be needed in the future? Just
curious.

https://codereview.appspot.com/7073051/

Revision history for this message
Gary Poster (gary) wrote :

On 2013/01/09 22:54:48, matthew.scott wrote:
> LGTM, thanks for the work-around. No changes here, but I do have a
question:
> will this eventually not be needed in the future? Just curious.

Once we do not need to support Precise, we can delete bootstrap_utils,
because we'll simply be able to apt-get install python-shelltoolbox,
import install_extra_repositories, and use it to get our PPA. Maybe in
the farther future even that will be unnecessary, but that's not clear
yet.

We will need to support Precise for years, I think.

Thank you for the review!

Gary

https://codereview.appspot.com/7073051/

Revision history for this message
Francesco Banconi (frankban) wrote :

Land as is.

Thanks for this workaround Gary.
This review is made by Nicola and me.

https://codereview.appspot.com/7073051/

23. By Gary Poster

increase the revision number

Revision history for this message
Gary Poster (gary) wrote :

*** Submitted:

Make charm work without environment Juju PPA

As discovered by therve and myself, the charm does not work if
"juju-origin: ppa" is not in your environments.yaml for the given Juju
environment. These changes work around the problem.

R=matthew.scott, frankban
CC=
https://codereview.appspot.com/7073051

https://codereview.appspot.com/7073051/

Preview Diff

Empty

Subscribers

People subscribed via source and target branches

to all changes: