Merge lp:~ted/pyjuju/bazaar-revision into lp:pyjuju

Proposed by Ted Gould
Status: Work in progress
Proposed branch: lp:~ted/pyjuju/bazaar-revision
Merge into: lp:pyjuju
Diff against target: 26 lines (+9/-0)
1 file modified
juju/charm/directory.py (+9/-0)
To merge this branch: bzr merge lp:~ted/pyjuju/bazaar-revision
Reviewer Review Type Date Requested Status
Gustavo Niemeyer Disapprove
Kapil Thangavelu (community) Disapprove
Review via email: mp+110596@code.launchpad.net

Description of the change

If there is no revision specified use the Bazaar revision if the charm is stored in a Bazaar repository.

I'm not sure how to test this, but I tried the code in ipython and it seems to work well enough. I'm posting mostly for feedback on the concept.

To post a comment you must log in.
Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Hi Ted.

I *love* this idea. I've always felt the explicit revision file was too clumsy.

I do think there is a strong desire to avoid tying to bzr for charms though, so you may see push back there.

Either way, this would need some documentation to go with it.

Revision history for this message
Kapil Thangavelu (hazmat) wrote :

-1 we shouldn't assume a local bzr installation to process a charm, this code will be used not just by the client, but also by any agent in the system when processing the charm. we have --upgrade flags in place to help satisfy the underlying use case here.

review: Disapprove
Revision history for this message
Ted Gould (ted) wrote :

On Sun, 2012-07-01 at 02:36 +0000, Kapil Thangavelu wrote:
> -1 we shouldn't assume a local bzr installation to process a charm,
> this code will be used not just by the client, but also by any agent
> in the system when processing the charm. we have --upgrade flags in
> place to help satisfy the underlying use case here.

I'm confused, this sounds like a good thing to me :-) Why would it be
an issue that everywhere on the system it would use the same version
number? Are you worried about Bazaar becoming a dependency of Juju?

The underlying requirement is to remove duplicate version information
from being stored in the charm files. I don't understand how a flag can
fix that use case.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

We already have an automatic revisioning system in the charm store, and as a relevant detail of how it's build, it actually cannot trust on the Bazaar revision, because that latter one can move backwards too. There are also other subtleties such as what to do when the same branch is linked from two different locations in Launchpad, such as the official charm and something else, or what to do when an official charm changed from one branch to the other (hint: we can't just pick the new branch revision, nor just inc. the previous revision).

For those reasons, this change isn't entirely adequate indeed.

review: Disapprove

Unmerged revisions

541. By Ted Gould

If there's no revision file try grabbing the revision from Bazaar if this is a repo

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'juju/charm/directory.py'
2--- juju/charm/directory.py 2012-04-04 11:26:08 +0000
3+++ juju/charm/directory.py 2012-06-15 18:47:22 +0000
4@@ -2,6 +2,7 @@
5 import stat
6 import zipfile
7 import tempfile
8+import subprocess
9
10 from juju.charm.base import CharmBase, get_revision
11 from juju.charm.bundle import CharmBundle
12@@ -31,6 +32,14 @@
13 if os.path.exists(revision_path):
14 with open(revision_path) as f:
15 revision_content = f.read()
16+ elif os.path.exists(os.path.join(self.path, ".bzr")):
17+ process = subprocess.Popen("bzr revno", shell=True, stdout=subprocess.PIPE)
18+ (revision_content, ) = process.communicate()
19+ retval = process.poll()
20+ if retval != 0:
21+ revision_content = None
22+ else:
23+ revision_content = revision_content.strip()
24 self._revision = get_revision(
25 revision_content, self.metadata, self.path)
26 if self._revision is None:

Subscribers

People subscribed via source and target branches

to status/vote changes: