Juju Charms Collection
glance-simplestreams-sync package

Merge lp://qastaging/~bjornt/charms/xenial/glance-simplestreams-sync/no-python2 into lp://qastaging/~landscape/charms/trusty/glance-simplestreams-sync/landscape

  1. Xenial Xerus (16.04)
  2. no-python2
  3. Merge into landscape
Proposed by Björn Tillenius
Status: Merged
Merged at revision: 69
Proposed branch: lp://qastaging/~bjornt/charms/xenial/glance-simplestreams-sync/no-python2
Merge into: lp://qastaging/~landscape/charms/trusty/glance-simplestreams-sync/landscape
Diff against target: 39 lines (+21/-1)
2 files modified
hooks/hooks.py (+1/-1)
hooks/install (+20/-0)
To merge this branch: bzr merge lp://qastaging/~bjornt/charms/xenial/glance-simplestreams-sync/no-python2
Reviewer Review Type Date Requested Status
Alberto Donato (community) Approve
Adam Collard (community) Approve
Review via email: mp+298203@code.qastaging.launchpad.net

Description of the change

Allow the charm to work on xenial.

Xenial doesn't have python2 installed by default, so it needs to be installed before the real install hook gets executed.

The pattern used is the same as for other openstack charms. The install script was copied from the glance charm.

To post a comment you must log in.
Revision history for this message
Adam Collard (adam-collard) wrote :

Why not just add python into the DEPS array, as it stands right now we'll attempt installation of python for every dep (wasteful)

That said, +1

review: Approve
Revision history for this message
Björn Tillenius (bjornt) wrote :

> Why not just add python into the DEPS array, as it stands right now we'll
> attempt installation of python for every dep (wasteful)
>
> That said, +1

It's bash, I don't want to change it :) I copied this from the keystone charm. I'd rather be consistent.

To be clear, python is not installed for every dependency. If you look closer, 'python' isn't explicitly installed, only 'python-$dep'.

Revision history for this message
Adam Collard (adam-collard) wrote :

> > Why not just add python into the DEPS array, as it stands right now we'll
> > attempt installation of python for every dep (wasteful)
> >
> > That said, +1
>
> It's bash, I don't want to change it :) I copied this from the keystone charm.
> I'd rather be consistent.
>
> To be clear, python is not installed for every dependency. If you look closer,
> 'python' isn't explicitly installed, only 'python-$dep'.

Ah, good point. Missed that!

Revision history for this message
Alberto Donato (ack) wrote :

+1

Small nit inline

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to all changes: