Merge lp:~fwereade/pyjuju/isolate-formula-revisions into lp:pyjuju
- isolate-formula-revisions
- Merge into trunk
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | Kapil Thangavelu | ||||
Approved revision: | 386 | ||||
Merged at revision: | 389 | ||||
Proposed branch: | lp:~fwereade/pyjuju/isolate-formula-revisions | ||||
Merge into: | lp:pyjuju | ||||
Prerequisite: | lp:~fwereade/pyjuju/check-latest-formulas | ||||
Diff against target: |
1114 lines (+346/-144) 43 files modified
examples/oneiric/mysql/metadata.yaml (+0/-1) examples/oneiric/mysql/revision (+1/-0) examples/oneiric/php/metadata.yaml (+0/-1) examples/oneiric/php/revision (+1/-0) examples/oneiric/wordpress/metadata.yaml (+0/-1) examples/oneiric/wordpress/revision (+1/-0) juju/agents/tests/test_machine.py (+1/-1) juju/agents/tests/test_unit.py (+2/-2) juju/charm/base.py (+31/-0) juju/charm/bundle.py (+19/-16) juju/charm/directory.py (+23/-4) juju/charm/metadata.py (+13/-6) juju/charm/repository.py (+5/-5) juju/charm/tests/__init__.py (+1/-1) juju/charm/tests/repository/series/dummy/metadata.yaml (+0/-1) juju/charm/tests/repository/series/dummy/revision (+1/-0) juju/charm/tests/repository/series/mysql-alternative/metadata.yaml (+0/-1) juju/charm/tests/repository/series/mysql-alternative/revision (+1/-0) juju/charm/tests/repository/series/mysql/metadata.yaml (+0/-1) juju/charm/tests/repository/series/mysql/revision (+1/-0) juju/charm/tests/repository/series/new/metadata.yaml (+0/-1) juju/charm/tests/repository/series/new/revision (+1/-0) juju/charm/tests/repository/series/old/metadata.yaml (+0/-1) juju/charm/tests/repository/series/old/revision (+1/-0) juju/charm/tests/repository/series/riak/metadata.yaml (+0/-1) juju/charm/tests/repository/series/riak/revision (+1/-0) juju/charm/tests/repository/series/varnish-alternative/metadata.yaml (+0/-1) juju/charm/tests/repository/series/varnish-alternative/revision (+1/-0) juju/charm/tests/repository/series/varnish/metadata.yaml (+0/-1) juju/charm/tests/repository/series/varnish/revision (+1/-0) juju/charm/tests/repository/series/wordpress/metadata.yaml (+0/-1) juju/charm/tests/repository/series/wordpress/revision (+1/-0) juju/charm/tests/test_base.py (+42/-16) juju/charm/tests/test_bundle.py (+60/-21) juju/charm/tests/test_directory.py (+62/-4) juju/charm/tests/test_metadata.py (+33/-19) juju/charm/tests/test_repository.py (+31/-24) juju/control/deploy.py (+1/-1) juju/control/tests/test_upgrade_charm.py (+8/-8) juju/machine/tests/test_unit_deployment.py (+1/-1) juju/state/charm.py (+0/-1) juju/state/tests/test_charm.py (+0/-1) juju/unit/tests/test_charm.py (+1/-1) |
||||
To merge this branch: | bzr merge lp:~fwereade/pyjuju/isolate-formula-revisions | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Kapil Thangavelu (community) | Approve | ||
Gustavo Niemeyer | Approve | ||
Review via email: mp+78164@code.launchpad.net |
Commit message
Description of the change
Notable points:
* CharmBundle.
* charms with revision still in metadata.yaml are log.warning()ed
* charms with revision in both places are log.warning()ed in the same way, and the standalone revision file (if valid) is treated as the single point of truth
* charms with no revision at all can't be initialised
* charm metadata prefers to get revision information by calling a function passed in at construction time, to accommodate actual Charms (whose revision could, in theory, change during the lifetime of a Charm object; the result should always be up to date), but will fall back to the value in _data if no getter is present (or sane)
* MetaData is responsible for the warning, which only triggers if we're going through .parse(); when we're storing charm state in ZK, we still need the revision (which is therefore included in get_serializati
* CharmDirectory will, upon initialization, create the appropriate revision file inside the charm's directory, so all the average charm author should have to do is delete one line from metadata.yaml; if they fail to do this, the apparent revision will be frozen even when they change metadata.yaml... BUT, as soon as local auto-updating lands (tomorrow morning(?)), this will be resolved, and they should be free from ever worrying about manually tracking revisions again
...phew. It's not that much code in the end, but I think it warrants fairly close and pedantic examination ;).
- 382. By William Reade
-
merge trunk
William Reade (fwereade) wrote : | # |
[1,2]
> Hmmm.. this is feeling a bit like a hack.
Guilty as charged, but I'd say it's quite well bounded.
> The storage and API in juju/state/charm.py will also have to change
> accordingly, for instance.
That's the core of the problem: that I thought I'd done enough damage in the last week or so, and that it would be important *not* to break charm-state storage and thus have to bump topology.VERSION. I'll go ahead with the nicer but more disruptive approach you propose in a stacked branch.
[3,4]
Good points; I'll fix them in this branch.
- 383. By William Reade
-
easy review points
William Reade (fwereade) wrote : | # |
> That's the core of the problem: that I thought I'd done enough damage in the
> last week or so, and that it would be important *not* to break charm-state
> storage and thus have to bump topology.VERSION. I'll go ahead with the nicer
> but more disruptive approach you propose in a stacked branch.
But... just possibly... we don't actually need .revision on metadata *at all*, because CharmState takes if from a CharmURL. Stacking it anyway, just to be safe.
- 384. By William Reade
-
merge trunk
William Reade (fwereade) wrote : | # |
Merged back from lp:~fwereade/juju/really-isolate-formula-revisions
MetaData no longer has .revision; just .obsolete_revision. Charms have .get_revision() to compensate, which uses metadata.
- 385. By William Reade
-
merge back from temporary really-
isolate- formula- revision branch
Gustavo Niemeyer (niemeyer) wrote : | # |
This looks fantastic. Just a few trivials, and let's try to get a +1 from Kapil.
[5]
+ revision = self.get_revision()
+ if revision is None:
+ raise CharmError(path, "has no revision")
Hmmm.. both implementations are doing this, so maybe the common
get_revision function itself could take care of it if neither
options have a valid revision.
[6]
+ revision = self.get_revision()
+ if revision is None:
+ raise CharmError(path, "has no revision")
+ if self._get_
+ self.set_
_get_revision_
least twice every time. Plus when the revision is actually
wanted outside of the implementation.
Maybe we should cache in a private attibute instead?
[7]
+ def set_revision(self, revision):
+ with open(os.
+ f.write(
A "\n" at the end would be nice to deal with it from the shell.
- 386. By William Reade
-
addressed review points
Kapil Thangavelu (hazmat) wrote : | # |
Looks good to me
- 387. By William Reade
-
strip file contents before int()ing (explicit > implicit)
- 388. By William Reade
-
just read charm revisions once, inline in __init__
Preview Diff
1 | === modified file 'examples/oneiric/mysql/metadata.yaml' | |||
2 | --- examples/oneiric/mysql/metadata.yaml 2011-09-15 19:24:47 +0000 | |||
3 | +++ examples/oneiric/mysql/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
4 | @@ -1,5 +1,4 @@ | |||
5 | 1 | name: mysql | 1 | name: mysql |
6 | 2 | revision: 11 | ||
7 | 3 | summary: "MySQL relational database provider" | 2 | summary: "MySQL relational database provider" |
8 | 4 | description: | | 3 | description: | |
9 | 5 | Installs and configures the MySQL package (mysqldb), then runs it. | 4 | Installs and configures the MySQL package (mysqldb), then runs it. |
10 | 6 | 5 | ||
11 | === added file 'examples/oneiric/mysql/revision' | |||
12 | --- examples/oneiric/mysql/revision 1970-01-01 00:00:00 +0000 | |||
13 | +++ examples/oneiric/mysql/revision 2011-10-05 18:13:26 +0000 | |||
14 | @@ -0,0 +1,1 @@ | |||
15 | 1 | 11 | ||
16 | 0 | 2 | ||
17 | === modified file 'examples/oneiric/php/metadata.yaml' | |||
18 | --- examples/oneiric/php/metadata.yaml 2011-09-15 19:24:47 +0000 | |||
19 | +++ examples/oneiric/php/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
20 | @@ -1,5 +1,4 @@ | |||
21 | 1 | name: php | 1 | name: php |
22 | 2 | revision: 5 | ||
23 | 3 | summary: "php container" | 2 | summary: "php container" |
24 | 4 | description: | | 3 | description: | |
25 | 5 | PHP environment for your code. | 4 | PHP environment for your code. |
26 | 6 | 5 | ||
27 | === added file 'examples/oneiric/php/revision' | |||
28 | --- examples/oneiric/php/revision 1970-01-01 00:00:00 +0000 | |||
29 | +++ examples/oneiric/php/revision 2011-10-05 18:13:26 +0000 | |||
30 | @@ -0,0 +1,1 @@ | |||
31 | 1 | 5 | ||
32 | 0 | 2 | ||
33 | === modified file 'examples/oneiric/wordpress/metadata.yaml' | |||
34 | --- examples/oneiric/wordpress/metadata.yaml 2011-09-15 19:24:47 +0000 | |||
35 | +++ examples/oneiric/wordpress/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
36 | @@ -1,5 +1,4 @@ | |||
37 | 1 | name: wordpress | 1 | name: wordpress |
38 | 2 | revision: 31 | ||
39 | 3 | summary: "WordPress blog" | 2 | summary: "WordPress blog" |
40 | 4 | description: | | 3 | description: | |
41 | 5 | Installs WordPress package (wordpress). Upon the database provider | 4 | Installs WordPress package (wordpress). Upon the database provider |
42 | 6 | 5 | ||
43 | === added file 'examples/oneiric/wordpress/revision' | |||
44 | --- examples/oneiric/wordpress/revision 1970-01-01 00:00:00 +0000 | |||
45 | +++ examples/oneiric/wordpress/revision 2011-10-05 18:13:26 +0000 | |||
46 | @@ -0,0 +1,1 @@ | |||
47 | 1 | 31 | ||
48 | 0 | 2 | ||
49 | === modified file 'juju/agents/tests/test_machine.py' | |||
50 | --- juju/agents/tests/test_machine.py 2011-09-28 09:48:30 +0000 | |||
51 | +++ juju/agents/tests/test_machine.py 2011-10-05 18:13:26 +0000 | |||
52 | @@ -170,7 +170,7 @@ | |||
53 | 170 | self.assertTrue(os.path.exists(charm_path)) | 170 | self.assertTrue(os.path.exists(charm_path)) |
54 | 171 | bundle = CharmBundle(charm_path) | 171 | bundle = CharmBundle(charm_path) |
55 | 172 | self.assertEquals( | 172 | self.assertEquals( |
57 | 173 | bundle.metadata.revision, self.charm.metadata.revision) | 173 | bundle.get_revision(), self.charm.get_revision()) |
58 | 174 | self.assertEquals(bundle.get_sha256(), checksum) | 174 | self.assertEquals(bundle.get_sha256(), checksum) |
59 | 175 | self.assertIn( | 175 | self.assertIn( |
60 | 176 | "Downloading charm %s" % charm_id, self.output.getvalue()) | 176 | "Downloading charm %s" % charm_id, self.output.getvalue()) |
61 | 177 | 177 | ||
62 | === modified file 'juju/agents/tests/test_unit.py' | |||
63 | --- juju/agents/tests/test_unit.py 2011-10-04 21:20:30 +0000 | |||
64 | +++ juju/agents/tests/test_unit.py 2011-10-05 18:13:26 +0000 | |||
65 | @@ -598,7 +598,7 @@ | |||
66 | 598 | os.path.join(self.agent.unit_directory, "charm")) | 598 | os.path.join(self.agent.unit_directory, "charm")) |
67 | 599 | 599 | ||
68 | 600 | self.assertEqual( | 600 | self.assertEqual( |
70 | 601 | self.charm.metadata.revision + 1, new_charm.metadata.revision) | 601 | self.charm.get_revision() + 1, new_charm.get_revision()) |
71 | 602 | 602 | ||
72 | 603 | @inlineCallbacks | 603 | @inlineCallbacks |
73 | 604 | def test_agent_upgrade_bad_unit_state(self): | 604 | def test_agent_upgrade_bad_unit_state(self): |
74 | @@ -694,7 +694,7 @@ | |||
75 | 694 | os.path.join(self.agent.unit_directory, "charm")) | 694 | os.path.join(self.agent.unit_directory, "charm")) |
76 | 695 | 695 | ||
77 | 696 | self.assertEqual( | 696 | self.assertEqual( |
79 | 697 | self.charm.metadata.revision + 1, new_charm.metadata.revision) | 697 | self.charm.get_revision() + 1, new_charm.get_revision()) |
80 | 698 | 698 | ||
81 | 699 | # Verify upgrade flag is cleared. | 699 | # Verify upgrade flag is cleared. |
82 | 700 | self.assertFalse((yield self.states["unit"].get_upgrade_flag())) | 700 | self.assertFalse((yield self.states["unit"].get_upgrade_flag())) |
83 | 701 | 701 | ||
84 | === modified file 'juju/charm/base.py' | |||
85 | --- juju/charm/base.py 2011-09-15 18:50:23 +0000 | |||
86 | +++ juju/charm/base.py 2011-10-05 18:13:26 +0000 | |||
87 | @@ -1,3 +1,20 @@ | |||
88 | 1 | from juju.errors import CharmError | ||
89 | 2 | |||
90 | 3 | |||
91 | 4 | def get_revision(file_content, metadata, path): | ||
92 | 5 | if file_content is None: | ||
93 | 6 | result = metadata.obsolete_revision | ||
94 | 7 | if result is None: | ||
95 | 8 | raise CharmError(path, "has no revision") | ||
96 | 9 | else: | ||
97 | 10 | message = "invalid charm revision %r" % file_content | ||
98 | 11 | try: | ||
99 | 12 | result = int(file_content.strip()) | ||
100 | 13 | except (ValueError, TypeError): | ||
101 | 14 | raise CharmError(path, message) | ||
102 | 15 | if result < 0: | ||
103 | 16 | raise CharmError(path, message) | ||
104 | 17 | return result | ||
105 | 1 | 18 | ||
106 | 2 | 19 | ||
107 | 3 | class CharmBase(object): | 20 | class CharmBase(object): |
108 | @@ -10,6 +27,20 @@ | |||
109 | 10 | raise NotImplementedError("%s.%s not supported" % | 27 | raise NotImplementedError("%s.%s not supported" % |
110 | 11 | (self.__class__.__name__, attr)) | 28 | (self.__class__.__name__, attr)) |
111 | 12 | 29 | ||
112 | 30 | def get_revision(self): | ||
113 | 31 | """Get the revision, preferably from the revision file. | ||
114 | 32 | |||
115 | 33 | Will fall back to metadata if not available. | ||
116 | 34 | """ | ||
117 | 35 | self._unsupported("get_revision()") | ||
118 | 36 | |||
119 | 37 | def set_revision(self, revision): | ||
120 | 38 | """Update the revision file, if possible. | ||
121 | 39 | |||
122 | 40 | Some subclasses may not be able to do this. | ||
123 | 41 | """ | ||
124 | 42 | self._unsupported("set_revision()") | ||
125 | 43 | |||
126 | 13 | def as_bundle(self): | 44 | def as_bundle(self): |
127 | 14 | """Transform this charm into a charm bundle, if possible. | 45 | """Transform this charm into a charm bundle, if possible. |
128 | 15 | 46 | ||
129 | 16 | 47 | ||
130 | === modified file 'juju/charm/bundle.py' | |||
131 | --- juju/charm/bundle.py 2011-09-15 18:50:23 +0000 | |||
132 | +++ juju/charm/bundle.py 2011-10-05 18:13:26 +0000 | |||
133 | @@ -4,7 +4,7 @@ | |||
134 | 4 | 4 | ||
135 | 5 | from zipfile import ZipFile, BadZipfile | 5 | from zipfile import ZipFile, BadZipfile |
136 | 6 | 6 | ||
138 | 7 | from juju.charm.base import CharmBase | 7 | from juju.charm.base import CharmBase, get_revision |
139 | 8 | from juju.charm.config import ConfigOptions | 8 | from juju.charm.config import ConfigOptions |
140 | 9 | from juju.charm.metadata import MetaData | 9 | from juju.charm.metadata import MetaData |
141 | 10 | from juju.errors import CharmError | 10 | from juju.errors import CharmError |
142 | @@ -15,28 +15,31 @@ | |||
143 | 15 | """ZIP-archive that contains charm directory content.""" | 15 | """ZIP-archive that contains charm directory content.""" |
144 | 16 | 16 | ||
145 | 17 | def __init__(self, path): | 17 | def __init__(self, path): |
146 | 18 | self.path = isinstance(path, file) and path.name or path | ||
147 | 18 | try: | 19 | try: |
148 | 19 | zf = ZipFile(path, 'r') | 20 | zf = ZipFile(path, 'r') |
149 | 20 | except BadZipfile, exc: | 21 | except BadZipfile, exc: |
150 | 21 | raise CharmError(path, "must be a zip file (%s)" % exc) | 22 | raise CharmError(path, "must be a zip file (%s)" % exc) |
151 | 22 | 23 | ||
152 | 23 | metadata = MetaData() | ||
153 | 24 | if "metadata.yaml" not in zf.namelist(): | 24 | if "metadata.yaml" not in zf.namelist(): |
163 | 25 | raise CharmError(path, | 25 | raise CharmError( |
164 | 26 | "archive does not contain required file " | 26 | path, "charm does not contain required file 'metadata.yaml'") |
165 | 27 | "\"metadata.yaml\"") | 27 | self.metadata = MetaData() |
166 | 28 | 28 | self.metadata.parse(zf.read("metadata.yaml")) | |
167 | 29 | content = zf.read("metadata.yaml") | 29 | |
168 | 30 | metadata.parse(content) | 30 | try: |
169 | 31 | self.metadata = metadata | 31 | revision_content = zf.read("revision") |
170 | 32 | 32 | except KeyError: | |
171 | 33 | config = ConfigOptions() | 33 | revision_content = None |
172 | 34 | self._revision = get_revision( | ||
173 | 35 | revision_content, self.metadata, self.path) | ||
174 | 36 | |||
175 | 37 | self.config = ConfigOptions() | ||
176 | 34 | if "config.yaml" in zf.namelist(): | 38 | if "config.yaml" in zf.namelist(): |
180 | 35 | content = zf.read("config.yaml") | 39 | self.config.parse(zf.read("config.yaml")) |
178 | 36 | config.parse(content) | ||
179 | 37 | self.config = config | ||
181 | 38 | 40 | ||
183 | 39 | self.path = isinstance(path, file) and path.name or path | 41 | def get_revision(self): |
184 | 42 | return self._revision | ||
185 | 40 | 43 | ||
186 | 41 | def compute_sha256(self): | 44 | def compute_sha256(self): |
187 | 42 | """Return the SHA256 digest for this charm bundle. | 45 | """Return the SHA256 digest for this charm bundle. |
188 | @@ -49,7 +52,7 @@ | |||
189 | 49 | """Extract the bundle to directory path and return a | 52 | """Extract the bundle to directory path and return a |
190 | 50 | CharmDirectory handle""" | 53 | CharmDirectory handle""" |
191 | 51 | from .directory import CharmDirectory | 54 | from .directory import CharmDirectory |
193 | 52 | zf = ZipFile(self.path, 'r') | 55 | zf = ZipFile(self.path, "r") |
194 | 53 | for info in zf.infolist(): | 56 | for info in zf.infolist(): |
195 | 54 | mode = info.external_attr >> 16 | 57 | mode = info.external_attr >> 16 |
196 | 55 | extract_path = zf.extract(info, directory_path) | 58 | extract_path = zf.extract(info, directory_path) |
197 | 56 | 59 | ||
198 | === modified file 'juju/charm/directory.py' | |||
199 | --- juju/charm/directory.py 2011-09-15 19:24:47 +0000 | |||
200 | +++ juju/charm/directory.py 2011-10-05 18:13:26 +0000 | |||
201 | @@ -2,10 +2,11 @@ | |||
202 | 2 | import zipfile | 2 | import zipfile |
203 | 3 | import tempfile | 3 | import tempfile |
204 | 4 | 4 | ||
205 | 5 | from juju.charm.base import CharmBase, get_revision | ||
206 | 6 | from juju.charm.bundle import CharmBundle | ||
207 | 5 | from juju.charm.config import ConfigOptions | 7 | from juju.charm.config import ConfigOptions |
208 | 6 | from juju.charm.metadata import MetaData | 8 | from juju.charm.metadata import MetaData |
211 | 7 | from juju.charm.bundle import CharmBundle | 9 | from juju.errors import CharmError |
210 | 8 | from juju.charm.base import CharmBase | ||
212 | 9 | 10 | ||
213 | 10 | 11 | ||
214 | 11 | class CharmDirectory(CharmBase): | 12 | class CharmDirectory(CharmBase): |
215 | @@ -22,11 +23,30 @@ | |||
216 | 22 | def __init__(self, path): | 23 | def __init__(self, path): |
217 | 23 | self.path = path | 24 | self.path = path |
218 | 24 | self.metadata = MetaData(os.path.join(path, "metadata.yaml")) | 25 | self.metadata = MetaData(os.path.join(path, "metadata.yaml")) |
219 | 26 | |||
220 | 27 | revision_content = None | ||
221 | 28 | revision_path = os.path.join(self.path, "revision") | ||
222 | 29 | if os.path.exists(revision_path): | ||
223 | 30 | with open(revision_path) as f: | ||
224 | 31 | revision_content = f.read() | ||
225 | 32 | self._revision = get_revision( | ||
226 | 33 | revision_content, self.metadata, self.path) | ||
227 | 34 | if revision_content is None: | ||
228 | 35 | self.set_revision(self._revision) | ||
229 | 36 | |||
230 | 25 | self.config = ConfigOptions() | 37 | self.config = ConfigOptions() |
231 | 26 | self.config.load(os.path.join(path, "config.yaml")) | 38 | self.config.load(os.path.join(path, "config.yaml")) |
232 | 27 | self._temp_bundle = None | 39 | self._temp_bundle = None |
233 | 28 | self._temp_bundle_file = None | 40 | self._temp_bundle_file = None |
234 | 29 | 41 | ||
235 | 42 | def get_revision(self): | ||
236 | 43 | return self._revision | ||
237 | 44 | |||
238 | 45 | def set_revision(self, revision): | ||
239 | 46 | self._revision = revision | ||
240 | 47 | with open(os.path.join(self.path, "revision"), "w") as f: | ||
241 | 48 | f.write(str(revision) + "\n") | ||
242 | 49 | |||
243 | 30 | def make_archive(self, path): | 50 | def make_archive(self, path): |
244 | 31 | """Create archive of directory and write to ``path``. | 51 | """Create archive of directory and write to ``path``. |
245 | 32 | 52 | ||
246 | @@ -60,8 +80,7 @@ | |||
247 | 60 | 80 | ||
248 | 61 | def as_bundle(self): | 81 | def as_bundle(self): |
249 | 62 | if self._temp_bundle is None: | 82 | if self._temp_bundle is None: |
252 | 63 | prefix = "%s-%d.charm." % \ | 83 | prefix = "%s-%d.charm." % (self.metadata.name, self.get_revision()) |
251 | 64 | (self.metadata.name, self.metadata.revision) | ||
253 | 65 | temp_file = tempfile.NamedTemporaryFile(prefix=prefix) | 84 | temp_file = tempfile.NamedTemporaryFile(prefix=prefix) |
254 | 66 | self.make_archive(temp_file.name) | 85 | self.make_archive(temp_file.name) |
255 | 67 | self._temp_bundle = CharmBundle(temp_file.name) | 86 | self._temp_bundle = CharmBundle(temp_file.name) |
256 | 68 | 87 | ||
257 | === modified file 'juju/charm/metadata.py' | |||
258 | --- juju/charm/metadata.py 2011-09-28 23:04:39 +0000 | |||
259 | +++ juju/charm/metadata.py 2011-10-05 18:13:26 +0000 | |||
260 | @@ -1,15 +1,17 @@ | |||
261 | 1 | import logging | ||
262 | 1 | import os | 2 | import os |
263 | 2 | 3 | ||
264 | 3 | import yaml | 4 | import yaml |
265 | 4 | 5 | ||
266 | 5 | from juju.errors import JujuError, FileNotFound | 6 | from juju.errors import JujuError, FileNotFound |
267 | 6 | from juju.charm.errors import CharmURLError | ||
268 | 7 | from juju.charm.url import CharmURL | ||
269 | 8 | from juju.lib.schema import ( | 7 | from juju.lib.schema import ( |
270 | 9 | SchemaError, Bool, Constant, Dict, Int, | 8 | SchemaError, Bool, Constant, Dict, Int, |
271 | 10 | KeyDict, OneOf, UnicodeOrString) | 9 | KeyDict, OneOf, UnicodeOrString) |
272 | 11 | 10 | ||
273 | 12 | 11 | ||
274 | 12 | log = logging.getLogger("juju.charm") | ||
275 | 13 | |||
276 | 14 | |||
277 | 13 | class MetaDataError(JujuError): | 15 | class MetaDataError(JujuError): |
278 | 14 | """Raised when an error in the info file of a charm is found.""" | 16 | """Raised when an error in the info file of a charm is found.""" |
279 | 15 | 17 | ||
280 | @@ -88,7 +90,7 @@ | |||
281 | 88 | "peers": Dict(UTF8_SCHEMA, InterfaceExpander(limit=1)), | 90 | "peers": Dict(UTF8_SCHEMA, InterfaceExpander(limit=1)), |
282 | 89 | "provides": Dict(UTF8_SCHEMA, InterfaceExpander(limit=None)), | 91 | "provides": Dict(UTF8_SCHEMA, InterfaceExpander(limit=None)), |
283 | 90 | "requires": Dict(UTF8_SCHEMA, InterfaceExpander(limit=1)), | 92 | "requires": Dict(UTF8_SCHEMA, InterfaceExpander(limit=1)), |
285 | 91 | }, optional=set(["provides", "requires", "peers"])) | 93 | }, optional=set(["provides", "requires", "peers", "revision"])) |
286 | 92 | 94 | ||
287 | 93 | 95 | ||
288 | 94 | class MetaData(object): | 96 | class MetaData(object): |
289 | @@ -110,12 +112,13 @@ | |||
290 | 110 | return self._data.get("name") | 112 | return self._data.get("name") |
291 | 111 | 113 | ||
292 | 112 | @property | 114 | @property |
294 | 113 | def revision(self): | 115 | def obsolete_revision(self): |
295 | 114 | """The charm revision. | 116 | """The charm revision. |
296 | 115 | 117 | ||
297 | 116 | The charm revision acts as a version, but unlike e.g. package | 118 | The charm revision acts as a version, but unlike e.g. package |
298 | 117 | versions, the charm revision is a monotonically increasing | 119 | versions, the charm revision is a monotonically increasing |
300 | 118 | integer. | 120 | integer. This should not be stored in metadata any more, but remains |
301 | 121 | for backward compatibility's sake. | ||
302 | 119 | """ | 122 | """ |
303 | 120 | return self._data.get("revision") | 123 | return self._data.get("revision") |
304 | 121 | 124 | ||
305 | @@ -174,7 +177,11 @@ | |||
306 | 174 | 177 | ||
307 | 175 | @raise MetaDataError: When errors are found in the info data. | 178 | @raise MetaDataError: When errors are found in the info data. |
308 | 176 | """ | 179 | """ |
310 | 177 | return self.parse_serialization_data(yaml.load(content), path) | 180 | self.parse_serialization_data(yaml.load(content), path) |
311 | 181 | if "revision" in self._data and path: | ||
312 | 182 | log.warning( | ||
313 | 183 | "%s: revision field is obsolete. Move it to the 'revision' " | ||
314 | 184 | "file." % path) | ||
315 | 178 | 185 | ||
316 | 179 | def parse_serialization_data(self, serialization_data, path=None): | 186 | def parse_serialization_data(self, serialization_data, path=None): |
317 | 180 | """Parse the unprocessed serialization data and load in this instance. | 187 | """Parse the unprocessed serialization data and load in this instance. |
318 | 181 | 188 | ||
319 | === modified file 'juju/charm/repository.py' | |||
320 | --- juju/charm/repository.py 2011-10-05 09:51:16 +0000 | |||
321 | +++ juju/charm/repository.py 2011-10-05 18:13:26 +0000 | |||
322 | @@ -61,20 +61,20 @@ | |||
323 | 61 | latest = None | 61 | latest = None |
324 | 62 | for charm in self._collection(charm_url.collection): | 62 | for charm in self._collection(charm_url.collection): |
325 | 63 | if charm.metadata.name == charm_url.name: | 63 | if charm.metadata.name == charm_url.name: |
327 | 64 | if charm.metadata.revision == charm_url.revision: | 64 | if charm.get_revision() == charm_url.revision: |
328 | 65 | return succeed(charm) | 65 | return succeed(charm) |
329 | 66 | if (latest is None or | 66 | if (latest is None or |
331 | 67 | latest.metadata.revision < charm.metadata.revision): | 67 | latest.get_revision() < charm.get_revision()): |
332 | 68 | latest = charm | 68 | latest = charm |
333 | 69 | 69 | ||
335 | 70 | if latest is None: | 70 | if latest is None or charm_url.revision is not None: |
336 | 71 | return fail(CharmNotFound(self.path, charm_url)) | 71 | return fail(CharmNotFound(self.path, charm_url)) |
337 | 72 | 72 | ||
338 | 73 | return succeed(latest) | 73 | return succeed(latest) |
339 | 74 | 74 | ||
340 | 75 | def latest(self, charm_url): | 75 | def latest(self, charm_url): |
341 | 76 | d = self.find(charm_url.with_revision(None)) | 76 | d = self.find(charm_url.with_revision(None)) |
343 | 77 | d.addCallback(lambda c: c.metadata.revision) | 77 | d.addCallback(lambda c: c.get_revision()) |
344 | 78 | return d | 78 | return d |
345 | 79 | 79 | ||
346 | 80 | 80 | ||
347 | @@ -128,7 +128,7 @@ | |||
348 | 128 | yield self._download(charm_url, cache_path) | 128 | yield self._download(charm_url, cache_path) |
349 | 129 | charm = get_charm_from_path(cache_path) | 129 | charm = get_charm_from_path(cache_path) |
350 | 130 | 130 | ||
352 | 131 | assert charm.metadata.revision == revision, "bad charm revision" | 131 | assert charm.get_revision() == revision, "bad charm revision" |
353 | 132 | if charm.get_sha256() != info["sha256"]: | 132 | if charm.get_sha256() != info["sha256"]: |
354 | 133 | os.remove(cache_path) | 133 | os.remove(cache_path) |
355 | 134 | name = "%s (%s)" % ( | 134 | name = "%s (%s)" % ( |
356 | 135 | 135 | ||
357 | === modified file 'juju/charm/tests/__init__.py' | |||
358 | --- juju/charm/tests/__init__.py 2011-09-28 00:36:30 +0000 | |||
359 | +++ juju/charm/tests/__init__.py 2011-10-05 18:13:26 +0000 | |||
360 | @@ -1,3 +1,3 @@ | |||
361 | 1 | def local_charm_id(charm): | 1 | def local_charm_id(charm): |
362 | 2 | return "local:series/%s-%s" % ( | 2 | return "local:series/%s-%s" % ( |
364 | 3 | charm.metadata.name, charm.metadata.revision) | 3 | charm.metadata.name, charm.get_revision()) |
365 | 4 | 4 | ||
366 | === modified file 'juju/charm/tests/repository/series/dummy/metadata.yaml' | |||
367 | --- juju/charm/tests/repository/series/dummy/metadata.yaml 2011-09-15 19:24:47 +0000 | |||
368 | +++ juju/charm/tests/repository/series/dummy/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
369 | @@ -1,5 +1,4 @@ | |||
370 | 1 | name: dummy | 1 | name: dummy |
371 | 2 | revision: 1 | ||
372 | 3 | summary: "That's a dummy charm." | 2 | summary: "That's a dummy charm." |
373 | 4 | description: | | 3 | description: | |
374 | 5 | This is a longer description which | 4 | This is a longer description which |
375 | 6 | 5 | ||
376 | === added file 'juju/charm/tests/repository/series/dummy/revision' | |||
377 | --- juju/charm/tests/repository/series/dummy/revision 1970-01-01 00:00:00 +0000 | |||
378 | +++ juju/charm/tests/repository/series/dummy/revision 2011-10-05 18:13:26 +0000 | |||
379 | @@ -0,0 +1,1 @@ | |||
380 | 1 | 1 | ||
381 | 0 | \ No newline at end of file | 2 | \ No newline at end of file |
382 | 1 | 3 | ||
383 | === modified file 'juju/charm/tests/repository/series/mysql-alternative/metadata.yaml' | |||
384 | --- juju/charm/tests/repository/series/mysql-alternative/metadata.yaml 2011-09-28 08:55:57 +0000 | |||
385 | +++ juju/charm/tests/repository/series/mysql-alternative/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
386 | @@ -1,5 +1,4 @@ | |||
387 | 1 | name: mysql-alternative | 1 | name: mysql-alternative |
388 | 2 | revision: 1 | ||
389 | 3 | summary: "Database engine" | 2 | summary: "Database engine" |
390 | 4 | description: "A pretty popular database" | 3 | description: "A pretty popular database" |
391 | 5 | provides: | 4 | provides: |
392 | 6 | 5 | ||
393 | === added file 'juju/charm/tests/repository/series/mysql-alternative/revision' | |||
394 | --- juju/charm/tests/repository/series/mysql-alternative/revision 1970-01-01 00:00:00 +0000 | |||
395 | +++ juju/charm/tests/repository/series/mysql-alternative/revision 2011-10-05 18:13:26 +0000 | |||
396 | @@ -0,0 +1,1 @@ | |||
397 | 1 | 1 | ||
398 | 0 | \ No newline at end of file | 2 | \ No newline at end of file |
399 | 1 | 3 | ||
400 | === modified file 'juju/charm/tests/repository/series/mysql/metadata.yaml' | |||
401 | --- juju/charm/tests/repository/series/mysql/metadata.yaml 2011-09-15 19:24:47 +0000 | |||
402 | +++ juju/charm/tests/repository/series/mysql/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
403 | @@ -1,5 +1,4 @@ | |||
404 | 1 | name: mysql | 1 | name: mysql |
405 | 2 | revision: 1 | ||
406 | 3 | summary: "Database engine" | 2 | summary: "Database engine" |
407 | 4 | description: "A pretty popular database" | 3 | description: "A pretty popular database" |
408 | 5 | provides: | 4 | provides: |
409 | 6 | 5 | ||
410 | === added file 'juju/charm/tests/repository/series/mysql/revision' | |||
411 | --- juju/charm/tests/repository/series/mysql/revision 1970-01-01 00:00:00 +0000 | |||
412 | +++ juju/charm/tests/repository/series/mysql/revision 2011-10-05 18:13:26 +0000 | |||
413 | @@ -0,0 +1,1 @@ | |||
414 | 1 | 1 | ||
415 | 0 | \ No newline at end of file | 2 | \ No newline at end of file |
416 | 1 | 3 | ||
417 | === modified file 'juju/charm/tests/repository/series/new/metadata.yaml' | |||
418 | --- juju/charm/tests/repository/series/new/metadata.yaml 2011-09-15 19:24:47 +0000 | |||
419 | +++ juju/charm/tests/repository/series/new/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
420 | @@ -1,5 +1,4 @@ | |||
421 | 1 | name: sample | 1 | name: sample |
422 | 2 | revision: 2 | ||
423 | 3 | summary: "That's a sample charm." | 2 | summary: "That's a sample charm." |
424 | 4 | description: | | 3 | description: | |
425 | 5 | This is a longer description which | 4 | This is a longer description which |
426 | 6 | 5 | ||
427 | === added file 'juju/charm/tests/repository/series/new/revision' | |||
428 | --- juju/charm/tests/repository/series/new/revision 1970-01-01 00:00:00 +0000 | |||
429 | +++ juju/charm/tests/repository/series/new/revision 2011-10-05 18:13:26 +0000 | |||
430 | @@ -0,0 +1,1 @@ | |||
431 | 1 | 2 | ||
432 | 0 | 2 | ||
433 | === modified file 'juju/charm/tests/repository/series/old/metadata.yaml' | |||
434 | --- juju/charm/tests/repository/series/old/metadata.yaml 2011-09-15 19:24:47 +0000 | |||
435 | +++ juju/charm/tests/repository/series/old/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
436 | @@ -1,5 +1,4 @@ | |||
437 | 1 | name: sample | 1 | name: sample |
438 | 2 | revision: 1 | ||
439 | 3 | summary: "That's a sample charm." | 2 | summary: "That's a sample charm." |
440 | 4 | description: | | 3 | description: | |
441 | 5 | This is a longer description which | 4 | This is a longer description which |
442 | 6 | 5 | ||
443 | === added file 'juju/charm/tests/repository/series/old/revision' | |||
444 | --- juju/charm/tests/repository/series/old/revision 1970-01-01 00:00:00 +0000 | |||
445 | +++ juju/charm/tests/repository/series/old/revision 2011-10-05 18:13:26 +0000 | |||
446 | @@ -0,0 +1,1 @@ | |||
447 | 1 | 1 | ||
448 | 0 | 2 | ||
449 | === modified file 'juju/charm/tests/repository/series/riak/metadata.yaml' | |||
450 | --- juju/charm/tests/repository/series/riak/metadata.yaml 2011-09-15 19:24:47 +0000 | |||
451 | +++ juju/charm/tests/repository/series/riak/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
452 | @@ -1,5 +1,4 @@ | |||
453 | 1 | name: riak | 1 | name: riak |
454 | 2 | revision: 7 | ||
455 | 3 | summary: "K/V storage engine" | 2 | summary: "K/V storage engine" |
456 | 4 | description: "Scalable K/V Store in Erlang with Clocks :-)" | 3 | description: "Scalable K/V Store in Erlang with Clocks :-)" |
457 | 5 | provides: | 4 | provides: |
458 | 6 | 5 | ||
459 | === added file 'juju/charm/tests/repository/series/riak/revision' | |||
460 | --- juju/charm/tests/repository/series/riak/revision 1970-01-01 00:00:00 +0000 | |||
461 | +++ juju/charm/tests/repository/series/riak/revision 2011-10-05 18:13:26 +0000 | |||
462 | @@ -0,0 +1,1 @@ | |||
463 | 1 | 7 | ||
464 | 0 | \ No newline at end of file | 2 | \ No newline at end of file |
465 | 1 | 3 | ||
466 | === modified file 'juju/charm/tests/repository/series/varnish-alternative/metadata.yaml' | |||
467 | --- juju/charm/tests/repository/series/varnish-alternative/metadata.yaml 2011-09-28 08:55:57 +0000 | |||
468 | +++ juju/charm/tests/repository/series/varnish-alternative/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
469 | @@ -1,5 +1,4 @@ | |||
470 | 1 | name: varnish-alternative | 1 | name: varnish-alternative |
471 | 2 | revision: 1 | ||
472 | 3 | summary: "Database engine" | 2 | summary: "Database engine" |
473 | 4 | description: "Another popular database" | 3 | description: "Another popular database" |
474 | 5 | provides: | 4 | provides: |
475 | 6 | 5 | ||
476 | === added file 'juju/charm/tests/repository/series/varnish-alternative/revision' | |||
477 | --- juju/charm/tests/repository/series/varnish-alternative/revision 1970-01-01 00:00:00 +0000 | |||
478 | +++ juju/charm/tests/repository/series/varnish-alternative/revision 2011-10-05 18:13:26 +0000 | |||
479 | @@ -0,0 +1,1 @@ | |||
480 | 1 | 1 | ||
481 | 0 | \ No newline at end of file | 2 | \ No newline at end of file |
482 | 1 | 3 | ||
483 | === modified file 'juju/charm/tests/repository/series/varnish/metadata.yaml' | |||
484 | --- juju/charm/tests/repository/series/varnish/metadata.yaml 2011-09-15 19:24:47 +0000 | |||
485 | +++ juju/charm/tests/repository/series/varnish/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
486 | @@ -1,5 +1,4 @@ | |||
487 | 1 | name: varnish | 1 | name: varnish |
488 | 2 | revision: 1 | ||
489 | 3 | summary: "Database engine" | 2 | summary: "Database engine" |
490 | 4 | description: "Another popular database" | 3 | description: "Another popular database" |
491 | 5 | provides: | 4 | provides: |
492 | 6 | 5 | ||
493 | === added file 'juju/charm/tests/repository/series/varnish/revision' | |||
494 | --- juju/charm/tests/repository/series/varnish/revision 1970-01-01 00:00:00 +0000 | |||
495 | +++ juju/charm/tests/repository/series/varnish/revision 2011-10-05 18:13:26 +0000 | |||
496 | @@ -0,0 +1,1 @@ | |||
497 | 1 | 1 | ||
498 | 0 | \ No newline at end of file | 2 | \ No newline at end of file |
499 | 1 | 3 | ||
500 | === modified file 'juju/charm/tests/repository/series/wordpress/metadata.yaml' | |||
501 | --- juju/charm/tests/repository/series/wordpress/metadata.yaml 2011-09-15 19:24:47 +0000 | |||
502 | +++ juju/charm/tests/repository/series/wordpress/metadata.yaml 2011-10-05 18:13:26 +0000 | |||
503 | @@ -1,5 +1,4 @@ | |||
504 | 1 | name: wordpress | 1 | name: wordpress |
505 | 2 | revision: 3 | ||
506 | 3 | summary: "Blog engine" | 2 | summary: "Blog engine" |
507 | 4 | description: "A pretty popular blog engine" | 3 | description: "A pretty popular blog engine" |
508 | 5 | provides: | 4 | provides: |
509 | 6 | 5 | ||
510 | === added file 'juju/charm/tests/repository/series/wordpress/revision' | |||
511 | --- juju/charm/tests/repository/series/wordpress/revision 1970-01-01 00:00:00 +0000 | |||
512 | +++ juju/charm/tests/repository/series/wordpress/revision 2011-10-05 18:13:26 +0000 | |||
513 | @@ -0,0 +1,1 @@ | |||
514 | 1 | 3 | ||
515 | 0 | \ No newline at end of file | 2 | \ No newline at end of file |
516 | 1 | 3 | ||
517 | === modified file 'juju/charm/tests/test_base.py' | |||
518 | --- juju/charm/tests/test_base.py 2011-09-15 18:50:23 +0000 | |||
519 | +++ juju/charm/tests/test_base.py 2011-10-05 18:13:26 +0000 | |||
520 | @@ -1,5 +1,9 @@ | |||
521 | 1 | import yaml | ||
522 | 2 | |||
523 | 3 | from juju.charm.base import CharmBase, get_revision | ||
524 | 4 | from juju.charm.metadata import MetaData | ||
525 | 5 | from juju.errors import CharmError | ||
526 | 1 | from juju.lib.testing import TestCase | 6 | from juju.lib.testing import TestCase |
527 | 2 | from juju.charm.base import CharmBase | ||
528 | 3 | 7 | ||
529 | 4 | 8 | ||
530 | 5 | class MyCharm(CharmBase): | 9 | class MyCharm(CharmBase): |
531 | @@ -20,21 +24,13 @@ | |||
532 | 20 | else: | 24 | else: |
533 | 21 | self.fail("MyCharm.%s didn't fail" % attr_name) | 25 | self.fail("MyCharm.%s didn't fail" % attr_name) |
534 | 22 | 26 | ||
550 | 23 | def test_as_bundle_is_unsupported(self): | 27 | def test_unsupported(self): |
551 | 24 | self.assertUnsupported(lambda: self.charm.as_bundle(), | 28 | self.assertUnsupported(self.charm.as_bundle, "as_bundle()") |
552 | 25 | "as_bundle()") | 29 | self.assertUnsupported(self.charm.get_sha256, "compute_sha256()") |
553 | 26 | 30 | self.assertUnsupported(self.charm.compute_sha256, "compute_sha256()") | |
554 | 27 | def test_compute_sha256_is_unsupported(self): | 31 | self.assertUnsupported(self.charm.get_revision, "get_revision()") |
555 | 28 | self.assertUnsupported(lambda: self.charm.compute_sha256(), | 32 | self.assertUnsupported( |
556 | 29 | "compute_sha256()") | 33 | lambda: self.charm.set_revision(1), "set_revision()") |
542 | 30 | |||
543 | 31 | def test_get_sha256_fails_if_not_preset(self): | ||
544 | 32 | """ | ||
545 | 33 | get_sha256() will internally call compute_sha256() if | ||
546 | 34 | the digest value hasn't been previously set. | ||
547 | 35 | """ | ||
548 | 36 | self.assertUnsupported(lambda: self.charm.get_sha256(), | ||
549 | 37 | "compute_sha256()") | ||
557 | 38 | 34 | ||
558 | 39 | def test_compute_and_cache_sha256(self): | 35 | def test_compute_and_cache_sha256(self): |
559 | 40 | """ | 36 | """ |
560 | @@ -53,3 +49,33 @@ | |||
561 | 53 | sha256 = ["anothervalue"] | 49 | sha256 = ["anothervalue"] |
562 | 54 | # Should still be the same, since the old one was cached. | 50 | # Should still be the same, since the old one was cached. |
563 | 55 | self.assertEquals(charm.get_sha256(), "mysha") | 51 | self.assertEquals(charm.get_sha256(), "mysha") |
564 | 52 | |||
565 | 53 | |||
566 | 54 | class GetRevisionTest(TestCase): | ||
567 | 55 | |||
568 | 56 | def assert_good_content(self, content, value): | ||
569 | 57 | self.assertEquals(get_revision(content, None, None), value) | ||
570 | 58 | |||
571 | 59 | def assert_bad_content(self, content): | ||
572 | 60 | err = self.assertRaises( | ||
573 | 61 | CharmError, get_revision, content, None, "path") | ||
574 | 62 | self.assertEquals( | ||
575 | 63 | str(err), | ||
576 | 64 | "Error processing 'path': invalid charm revision %r" % content) | ||
577 | 65 | |||
578 | 66 | def test_with_content(self): | ||
579 | 67 | self.assert_good_content("0\n", 0) | ||
580 | 68 | self.assert_good_content("123\n", 123) | ||
581 | 69 | self.assert_bad_content("") | ||
582 | 70 | self.assert_bad_content("-1\n") | ||
583 | 71 | self.assert_bad_content("three hundred and six or so") | ||
584 | 72 | |||
585 | 73 | def test_metadata_fallback(self): | ||
586 | 74 | metadata = MetaData() | ||
587 | 75 | err = self.assertRaises( | ||
588 | 76 | CharmError, get_revision, None, metadata, "path") | ||
589 | 77 | self.assertEquals( | ||
590 | 78 | str(err), "Error processing 'path': has no revision") | ||
591 | 79 | metadata.parse(yaml.dump({ | ||
592 | 80 | "name": "x", "summary": "y", "description": "z","revision": 33})) | ||
593 | 81 | self.assertEquals(get_revision(None, metadata, None), 33) | ||
594 | 56 | 82 | ||
595 | === modified file 'juju/charm/tests/test_bundle.py' | |||
596 | --- juju/charm/tests/test_bundle.py 2011-09-28 08:55:57 +0000 | |||
597 | +++ juju/charm/tests/test_bundle.py 2011-10-05 18:13:26 +0000 | |||
598 | @@ -1,7 +1,7 @@ | |||
599 | 1 | import os | 1 | import os |
600 | 2 | import stat | ||
601 | 3 | import hashlib | 2 | import hashlib |
602 | 4 | import inspect | 3 | import inspect |
603 | 4 | import yaml | ||
604 | 5 | import zipfile | 5 | import zipfile |
605 | 6 | 6 | ||
606 | 7 | from juju.lib.testing import TestCase | 7 | from juju.lib.testing import TestCase |
607 | @@ -35,16 +35,11 @@ | |||
608 | 35 | 35 | ||
609 | 36 | def test_error_not_zip(self): | 36 | def test_error_not_zip(self): |
610 | 37 | filename = self.makeFile("@#$@$") | 37 | filename = self.makeFile("@#$@$") |
621 | 38 | try: | 38 | err = self.assertRaises(CharmError, CharmBundle, filename) |
622 | 39 | CharmBundle(filename) | 39 | self.assertEquals( |
623 | 40 | except CharmError, exc: | 40 | str(err), |
624 | 41 | self.assertEquals( | 41 | "Error processing %r: must be a zip file (File is not a zip file)" |
625 | 42 | str(exc), | 42 | % filename) |
616 | 43 | ("Error processing %r: " % filename) + | ||
617 | 44 | "must be a zip file (File is not a zip file)") | ||
618 | 45 | return | ||
619 | 46 | |||
620 | 47 | self.fail("Expected charm error.") | ||
626 | 48 | 43 | ||
627 | 49 | def test_error_zip_but_doesnt_have_metadata_file(self): | 44 | def test_error_zip_but_doesnt_have_metadata_file(self): |
628 | 50 | filename = self.makeFile() | 45 | filename = self.makeFile() |
629 | @@ -52,16 +47,60 @@ | |||
630 | 52 | zf.writestr("README.txt", "This is not a valid charm.") | 47 | zf.writestr("README.txt", "This is not a valid charm.") |
631 | 53 | zf.close() | 48 | zf.close() |
632 | 54 | 49 | ||
643 | 55 | try: | 50 | err = self.assertRaises(CharmError, CharmBundle, filename) |
644 | 56 | CharmBundle(filename) | 51 | self.assertEquals( |
645 | 57 | except CharmError, exc: | 52 | str(err), |
646 | 58 | self.assertEquals( | 53 | "Error processing %r: charm does not contain required " |
647 | 59 | str(exc), | 54 | "file 'metadata.yaml'" % filename) |
648 | 60 | ("Error processing %r: " % filename) + | 55 | |
649 | 61 | "archive does not contain required file \"metadata.yaml\"") | 56 | def test_no_revision_at_all(self): |
650 | 62 | return | 57 | filename = self.makeFile() |
651 | 63 | 58 | zf_dst = zipfile.ZipFile(filename, "w") | |
652 | 64 | self.fail("Expected charm error.") | 59 | zf_src = zipfile.ZipFile(self.filename, "r") |
653 | 60 | for name in zf_src.namelist(): | ||
654 | 61 | if name == "revision": | ||
655 | 62 | continue | ||
656 | 63 | zf_dst.writestr(name, zf_src.read(name)) | ||
657 | 64 | zf_src.close() | ||
658 | 65 | zf_dst.close() | ||
659 | 66 | |||
660 | 67 | err = self.assertRaises(CharmError, CharmBundle, filename) | ||
661 | 68 | self.assertEquals( | ||
662 | 69 | str(err), "Error processing %r: has no revision" % filename) | ||
663 | 70 | |||
664 | 71 | def test_revision_in_metadata(self): | ||
665 | 72 | filename = self.makeFile() | ||
666 | 73 | zf_dst = zipfile.ZipFile(filename, "w") | ||
667 | 74 | zf_src = zipfile.ZipFile(self.filename, "r") | ||
668 | 75 | for name in zf_src.namelist(): | ||
669 | 76 | if name == "revision": | ||
670 | 77 | continue | ||
671 | 78 | content = zf_src.read(name) | ||
672 | 79 | if name == "metadata.yaml": | ||
673 | 80 | data = yaml.load(content) | ||
674 | 81 | data["revision"] = 303 | ||
675 | 82 | content = yaml.dump(data) | ||
676 | 83 | zf_dst.writestr(name, content) | ||
677 | 84 | zf_src.close() | ||
678 | 85 | zf_dst.close() | ||
679 | 86 | |||
680 | 87 | charm = CharmBundle(filename) | ||
681 | 88 | self.assertEquals(charm.get_revision(), 303) | ||
682 | 89 | |||
683 | 90 | def test_competing_revisions(self): | ||
684 | 91 | zf = zipfile.ZipFile(self.filename, "a") | ||
685 | 92 | zf.writestr("revision", "999") | ||
686 | 93 | data = yaml.load(zf.read("metadata.yaml")) | ||
687 | 94 | data["revision"] = 303 | ||
688 | 95 | zf.writestr("metadata.yaml", yaml.dump(data)) | ||
689 | 96 | zf.close() | ||
690 | 97 | |||
691 | 98 | charm = CharmBundle(self.filename) | ||
692 | 99 | self.assertEquals(charm.get_revision(), 999) | ||
693 | 100 | |||
694 | 101 | def test_cannot_set_revision(self): | ||
695 | 102 | charm = CharmBundle(self.filename) | ||
696 | 103 | self.assertRaises(NotImplementedError, charm.set_revision, 123) | ||
697 | 65 | 104 | ||
698 | 66 | def test_bundled_config(self): | 105 | def test_bundled_config(self): |
699 | 67 | """Make sure that config is accessible from a bundle.""" | 106 | """Make sure that config is accessible from a bundle.""" |
700 | 68 | 107 | ||
701 | === modified file 'juju/charm/tests/test_directory.py' | |||
702 | --- juju/charm/tests/test_directory.py 2011-09-28 10:38:16 +0000 | |||
703 | +++ juju/charm/tests/test_directory.py 2011-10-05 18:13:26 +0000 | |||
704 | @@ -1,11 +1,13 @@ | |||
705 | 1 | import gc | ||
706 | 1 | import os | 2 | import os |
707 | 2 | import hashlib | 3 | import hashlib |
708 | 3 | import inspect | 4 | import inspect |
709 | 5 | import shutil | ||
710 | 4 | import tempfile | 6 | import tempfile |
711 | 7 | import yaml | ||
712 | 5 | import zipfile | 8 | import zipfile |
713 | 6 | import gc | ||
714 | 7 | 9 | ||
716 | 8 | from juju.errors import FileNotFound | 10 | from juju.errors import CharmError, FileNotFound |
717 | 9 | from juju.charm.metadata import MetaData | 11 | from juju.charm.metadata import MetaData |
718 | 10 | from juju.charm.directory import CharmDirectory | 12 | from juju.charm.directory import CharmDirectory |
719 | 11 | from juju.charm.bundle import CharmBundle | 13 | from juju.charm.bundle import CharmBundle |
720 | @@ -31,10 +33,62 @@ | |||
721 | 31 | if not os.path.isdir(empty_dir): | 33 | if not os.path.isdir(empty_dir): |
722 | 32 | os.mkdir(empty_dir) | 34 | os.mkdir(empty_dir) |
723 | 33 | 35 | ||
724 | 36 | def copy_charm(self): | ||
725 | 37 | dir_ = os.path.join(tempfile.mkdtemp(), "sample") | ||
726 | 38 | shutil.copytree(sample_directory, dir_) | ||
727 | 39 | return dir_ | ||
728 | 40 | |||
729 | 41 | def delete_revision(self, dir_): | ||
730 | 42 | os.remove(os.path.join(dir_, "revision")) | ||
731 | 43 | |||
732 | 44 | def set_metadata_revision(self, dir_, revision): | ||
733 | 45 | metadata_path = os.path.join(dir_, "metadata.yaml") | ||
734 | 46 | with open(metadata_path) as f: | ||
735 | 47 | data = yaml.load(f.read()) | ||
736 | 48 | data["revision"] = 999 | ||
737 | 49 | with open(metadata_path, "w") as f: | ||
738 | 50 | f.write(yaml.dump(data)) | ||
739 | 51 | |||
740 | 34 | def test_metadata_is_required(self): | 52 | def test_metadata_is_required(self): |
741 | 35 | directory = self.makeDir() | 53 | directory = self.makeDir() |
742 | 36 | self.assertRaises(FileNotFound, CharmDirectory, directory) | 54 | self.assertRaises(FileNotFound, CharmDirectory, directory) |
743 | 37 | 55 | ||
744 | 56 | def test_no_revision(self): | ||
745 | 57 | dir_ = self.copy_charm() | ||
746 | 58 | self.delete_revision(dir_) | ||
747 | 59 | err = self.assertRaises(CharmError, CharmDirectory, dir_) | ||
748 | 60 | self.assertEquals( | ||
749 | 61 | str(err), "Error processing %r: has no revision" % dir_) | ||
750 | 62 | |||
751 | 63 | def test_revision_in_metadata(self): | ||
752 | 64 | dir_ = self.copy_charm() | ||
753 | 65 | self.delete_revision(dir_) | ||
754 | 66 | self.set_metadata_revision(dir_, 999) | ||
755 | 67 | log = self.capture_logging("juju.charm") | ||
756 | 68 | charm = CharmDirectory(dir_) | ||
757 | 69 | self.assertEquals(charm.get_revision(), 999) | ||
758 | 70 | self.assertIn( | ||
759 | 71 | "revision field is obsolete. Move it to the 'revision' file.", | ||
760 | 72 | log.getvalue()) | ||
761 | 73 | |||
762 | 74 | def test_competing_revisions(self): | ||
763 | 75 | dir_ = self.copy_charm() | ||
764 | 76 | self.set_metadata_revision(dir_, 999) | ||
765 | 77 | log = self.capture_logging("juju.charm") | ||
766 | 78 | charm = CharmDirectory(dir_) | ||
767 | 79 | self.assertEquals(charm.get_revision(), 1) | ||
768 | 80 | self.assertIn( | ||
769 | 81 | "revision field is obsolete. Move it to the 'revision' file.", | ||
770 | 82 | log.getvalue()) | ||
771 | 83 | |||
772 | 84 | def test_set_revision(self): | ||
773 | 85 | dir_ = self.copy_charm() | ||
774 | 86 | charm = CharmDirectory(dir_) | ||
775 | 87 | charm.set_revision(123) | ||
776 | 88 | self.assertEquals(charm.get_revision(), 123) | ||
777 | 89 | with open(os.path.join(dir_, "revision")) as f: | ||
778 | 90 | self.assertEquals(f.read(), "123\n") | ||
779 | 91 | |||
780 | 38 | def test_info(self): | 92 | def test_info(self): |
781 | 39 | directory = CharmDirectory(sample_directory) | 93 | directory = CharmDirectory(sample_directory) |
782 | 40 | self.assertTrue(directory.metadata is not None) | 94 | self.assertTrue(directory.metadata is not None) |
783 | @@ -57,7 +111,7 @@ | |||
784 | 57 | self.assertEqual( | 111 | self.assertEqual( |
785 | 58 | set(included), | 112 | set(included), |
786 | 59 | set(("metadata.yaml", "empty/", "src/", "src/hello.c", | 113 | set(("metadata.yaml", "empty/", "src/", "src/hello.c", |
788 | 60 | "config.yaml", "hooks/", "hooks/install"))) | 114 | "config.yaml", "hooks/", "hooks/install", "revision"))) |
789 | 61 | 115 | ||
790 | 62 | def test_as_bundle(self): | 116 | def test_as_bundle(self): |
791 | 63 | directory = CharmDirectory(self.sample_dir1) | 117 | directory = CharmDirectory(self.sample_dir1) |
792 | @@ -66,10 +120,14 @@ | |||
793 | 66 | self.assertEquals(charm_bundle.metadata.name, "sample") | 120 | self.assertEquals(charm_bundle.metadata.name, "sample") |
794 | 67 | self.assertIn("sample-1.charm", charm_bundle.path) | 121 | self.assertIn("sample-1.charm", charm_bundle.path) |
795 | 68 | 122 | ||
796 | 123 | total_compressed = 0 | ||
797 | 124 | total_uncompressed = 0 | ||
798 | 69 | zip_file = zipfile.ZipFile(charm_bundle.path) | 125 | zip_file = zipfile.ZipFile(charm_bundle.path) |
799 | 70 | for n in zip_file.namelist(): | 126 | for n in zip_file.namelist(): |
800 | 71 | info = zip_file.getinfo(n) | 127 | info = zip_file.getinfo(n) |
802 | 72 | self.assertTrue(info.compress_size < info.file_size) | 128 | total_compressed += info.compress_size |
803 | 129 | total_uncompressed += info.file_size | ||
804 | 130 | self.assertTrue(total_compressed < total_uncompressed) | ||
805 | 73 | 131 | ||
806 | 74 | def test_as_bundle_file_lifetime(self): | 132 | def test_as_bundle_file_lifetime(self): |
807 | 75 | """ | 133 | """ |
808 | 76 | 134 | ||
809 | === modified file 'juju/charm/tests/test_metadata.py' | |||
810 | --- juju/charm/tests/test_metadata.py 2011-09-28 23:04:39 +0000 | |||
811 | +++ juju/charm/tests/test_metadata.py 2011-10-05 18:13:26 +0000 | |||
812 | @@ -5,7 +5,6 @@ | |||
813 | 5 | import inspect | 5 | import inspect |
814 | 6 | 6 | ||
815 | 7 | from juju.charm import tests | 7 | from juju.charm import tests |
816 | 8 | from juju.charm.errors import CharmURLError | ||
817 | 9 | from juju.charm.metadata import ( | 8 | from juju.charm.metadata import ( |
818 | 10 | MetaData, MetaDataError, InterfaceExpander, SchemaError) | 9 | MetaData, MetaDataError, InterfaceExpander, SchemaError) |
819 | 11 | from juju.errors import FileNotFound | 10 | from juju.errors import FileNotFound |
820 | @@ -57,7 +56,7 @@ | |||
821 | 57 | Attributes should be set to None before anything is loaded. | 56 | Attributes should be set to None before anything is loaded. |
822 | 58 | """ | 57 | """ |
823 | 59 | self.assertEquals(self.metadata.name, None) | 58 | self.assertEquals(self.metadata.name, None) |
825 | 60 | self.assertEquals(self.metadata.revision, None) | 59 | self.assertEquals(self.metadata.obsolete_revision, None) |
826 | 61 | self.assertEquals(self.metadata.summary, None) | 60 | self.assertEquals(self.metadata.summary, None) |
827 | 62 | self.assertEquals(self.metadata.description, None) | 61 | self.assertEquals(self.metadata.description, None) |
828 | 63 | 62 | ||
829 | @@ -68,11 +67,38 @@ | |||
830 | 68 | """ | 67 | """ |
831 | 69 | self.metadata.parse(self.sample) | 68 | self.metadata.parse(self.sample) |
832 | 70 | self.assertEquals(self.metadata.name, "dummy") | 69 | self.assertEquals(self.metadata.name, "dummy") |
838 | 71 | self.assertEquals(self.metadata.revision, 1) | 70 | self.assertEquals(self.metadata.obsolete_revision, None) |
839 | 72 | self.assertEquals(self.metadata.summary, u"That's a dummy charm.") | 71 | self.assertEquals(self.metadata.summary, u"That's a dummy charm.") |
840 | 73 | self.assertEquals(self.metadata.description, | 72 | self.assertEquals(self.metadata.description, |
841 | 74 | u"This is a longer description which\n" | 73 | u"This is a longer description which\n" |
842 | 75 | u"potentially contains multiple lines.\n") | 74 | u"potentially contains multiple lines.\n") |
843 | 75 | |||
844 | 76 | def assert_parse_with_revision(self, with_path): | ||
845 | 77 | """ | ||
846 | 78 | Parsing the content file should work. :-) Basic information will | ||
847 | 79 | be available as attributes of the info file. | ||
848 | 80 | """ | ||
849 | 81 | with self.change_sample() as data: | ||
850 | 82 | data["revision"] = 123 | ||
851 | 83 | log = self.capture_logging("juju.charm") | ||
852 | 84 | self.metadata.parse(self.sample, "some/path" if with_path else None) | ||
853 | 85 | if with_path: | ||
854 | 86 | self.assertIn( | ||
855 | 87 | "some/path: revision field is obsolete. Move it to the " | ||
856 | 88 | "'revision' file.", | ||
857 | 89 | log.getvalue()) | ||
858 | 90 | self.assertEquals(self.metadata.name, "dummy") | ||
859 | 91 | self.assertEquals(self.metadata.obsolete_revision, 123) | ||
860 | 92 | self.assertEquals(self.metadata.summary, u"That's a dummy charm.") | ||
861 | 93 | self.assertEquals(self.metadata.description, | ||
862 | 94 | u"This is a longer description which\n" | ||
863 | 95 | u"potentially contains multiple lines.\n") | ||
864 | 96 | self.assertEquals( | ||
865 | 97 | self.metadata.get_serialization_data()["revision"], 123) | ||
866 | 98 | |||
867 | 99 | def test_parse_with_revision(self): | ||
868 | 100 | self.assert_parse_with_revision(True) | ||
869 | 101 | self.assert_parse_with_revision(False) | ||
870 | 76 | 102 | ||
871 | 77 | def test_load_calls_parse_calls_parse_serialzation_data(self): | 103 | def test_load_calls_parse_calls_parse_serialzation_data(self): |
872 | 78 | """ | 104 | """ |
873 | @@ -118,18 +144,6 @@ | |||
874 | 118 | self.metadata.load, filename) | 144 | self.metadata.load, filename) |
875 | 119 | self.assertEquals(error.path, filename) | 145 | self.assertEquals(error.path, filename) |
876 | 120 | 146 | ||
877 | 121 | def test_revision_is_int(self): | ||
878 | 122 | """ | ||
879 | 123 | Revision numbers are *integers*. Yep, no 1.2.3 versioning. | ||
880 | 124 | """ | ||
881 | 125 | with self.change_sample() as data: | ||
882 | 126 | data["revision"] = "1" | ||
883 | 127 | error = self.assertRaises(MetaDataError, | ||
884 | 128 | self.metadata.parse, self.sample) | ||
885 | 129 | self.assertEquals(str(error), | ||
886 | 130 | "Bad data in charm info: revision: " | ||
887 | 131 | "expected int, got '1'") | ||
888 | 132 | |||
889 | 133 | def test_name_summary_and_description_are_utf8(self): | 147 | def test_name_summary_and_description_are_utf8(self): |
890 | 134 | """ | 148 | """ |
891 | 135 | Textual fields are decoded to unicode by the schema using UTF-8. | 149 | Textual fields are decoded to unicode by the schema using UTF-8. |
892 | 136 | 150 | ||
893 | === modified file 'juju/charm/tests/test_repository.py' | |||
894 | --- juju/charm/tests/test_repository.py 2011-10-05 09:51:16 +0000 | |||
895 | +++ juju/charm/tests/test_repository.py 2011-10-05 18:13:26 +0000 | |||
896 | @@ -63,16 +63,15 @@ | |||
897 | 63 | return CharmURL.parse("local:series/" + name) | 63 | return CharmURL.parse("local:series/" + name) |
898 | 64 | 64 | ||
899 | 65 | @inlineCallbacks | 65 | @inlineCallbacks |
901 | 66 | def assert_not_there(self, name): | 66 | def assert_not_there(self, name, repo, revision=None): |
902 | 67 | url = self.charm_url(name) | 67 | url = self.charm_url(name) |
903 | 68 | msg = "Charm 'local:series/%s' not found in repository %s" % ( | 68 | msg = "Charm 'local:series/%s' not found in repository %s" % ( |
911 | 69 | name, self.unbundled_repo_path) | 69 | name, repo.path) |
912 | 70 | err = yield self.assertFailure( | 70 | err = yield self.assertFailure(repo.find(url), CharmNotFound) |
913 | 71 | self.repository1.find(url), CharmNotFound) | 71 | self.assertEquals(str(err), msg) |
914 | 72 | self.assertEquals(str(err), msg) | 72 | if revision is None: |
915 | 73 | err = yield self.assertFailure( | 73 | err = yield self.assertFailure(repo.latest(url), CharmNotFound) |
916 | 74 | self.repository1.latest(url), CharmNotFound) | 74 | self.assertEquals(str(err), msg) |
910 | 75 | self.assertEquals(str(err), msg) | ||
917 | 76 | 75 | ||
918 | 77 | def test_find_inappropriate_url(self): | 76 | def test_find_inappropriate_url(self): |
919 | 78 | url = CharmURL.parse("cs:foo/bar") | 77 | url = CharmURL.parse("cs:foo/bar") |
920 | @@ -80,18 +79,18 @@ | |||
921 | 80 | self.assertEquals(str(err), "schema mismatch") | 79 | self.assertEquals(str(err), "schema mismatch") |
922 | 81 | 80 | ||
923 | 82 | def test_completely_missing(self): | 81 | def test_completely_missing(self): |
925 | 83 | return self.assert_not_there("zebra") | 82 | return self.assert_not_there("zebra", self.repository1) |
926 | 84 | 83 | ||
927 | 85 | def test_unkown_files_ignored(self): | 84 | def test_unkown_files_ignored(self): |
928 | 86 | self.makeFile( | 85 | self.makeFile( |
929 | 87 | "Foobar", | 86 | "Foobar", |
930 | 88 | path=os.path.join(self.repository1.path, "series", "zebra")) | 87 | path=os.path.join(self.repository1.path, "series", "zebra")) |
932 | 89 | return self.assert_not_there("zebra") | 88 | return self.assert_not_there("zebra", self.repository1) |
933 | 90 | 89 | ||
934 | 91 | def test_unknown_directories_ignored(self): | 90 | def test_unknown_directories_ignored(self): |
935 | 92 | self.makeDir( | 91 | self.makeDir( |
936 | 93 | path=os.path.join(self.repository1.path, "series", "zebra")) | 92 | path=os.path.join(self.repository1.path, "series", "zebra")) |
938 | 94 | return self.assert_not_there("zebra") | 93 | return self.assert_not_there("zebra", self.repository1) |
939 | 95 | 94 | ||
940 | 96 | def test_broken_charms_ignored(self): | 95 | def test_broken_charms_ignored(self): |
941 | 97 | charm_path = self.makeDir( | 96 | charm_path = self.makeDir( |
942 | @@ -104,37 +103,45 @@ | |||
943 | 104 | revision: 0 | 103 | revision: 0 |
944 | 105 | summary: hola""") | 104 | summary: hola""") |
945 | 106 | fh.close() | 105 | fh.close() |
947 | 107 | return self.assert_not_there("zebra") | 106 | return self.assert_not_there("zebra", self.repository1) |
948 | 108 | 107 | ||
949 | 109 | def assert_there(self, name, repo, revision, latest_revision=None): | 108 | def assert_there(self, name, repo, revision, latest_revision=None): |
950 | 110 | url = self.charm_url(name) | 109 | url = self.charm_url(name) |
951 | 111 | charm = yield repo.find(url) | 110 | charm = yield repo.find(url) |
953 | 112 | self.assertEquals(charm.metadata.revision, revision) | 111 | self.assertEquals(charm.get_revision(), revision) |
954 | 113 | latest = yield repo.latest(url) | 112 | latest = yield repo.latest(url) |
955 | 114 | self.assertEquals(latest, latest_revision or revision) | 113 | self.assertEquals(latest, latest_revision or revision) |
956 | 115 | 114 | ||
957 | 115 | @inlineCallbacks | ||
958 | 116 | def test_success_unbundled(self): | 116 | def test_success_unbundled(self): |
961 | 117 | return self.assert_there("sample", self.repository1, 2) | 117 | yield self.assert_there("sample", self.repository1, 2) |
962 | 118 | return self.assert_there("sample-2", self.repository1, 2) | 118 | yield self.assert_there("sample-1", self.repository1, 1, 2) |
963 | 119 | yield self.assert_there("sample-2", self.repository1, 2) | ||
964 | 120 | yield self.assert_not_there("sample-3", self.repository1, 2) | ||
965 | 119 | 121 | ||
966 | 122 | @inlineCallbacks | ||
967 | 120 | def test_success_bundled(self): | 123 | def test_success_bundled(self): |
970 | 121 | return self.assert_there("sample", self.repository2, 2) | 124 | yield self.assert_there("sample", self.repository2, 2) |
971 | 122 | return self.assert_there("sample-2", self.repository2, 2) | 125 | yield self.assert_there("sample-1", self.repository2, 1, 2) |
972 | 126 | yield self.assert_there("sample-2", self.repository2, 2) | ||
973 | 127 | yield self.assert_not_there("sample-3", self.repository2, 2) | ||
974 | 123 | 128 | ||
975 | 124 | @inlineCallbacks | 129 | @inlineCallbacks |
976 | 125 | def test_no_revision_gets_latest(self): | 130 | def test_no_revision_gets_latest(self): |
977 | 126 | yield self.assert_there("sample", self.repository1, 2) | 131 | yield self.assert_there("sample", self.repository1, 2) |
978 | 132 | yield self.assert_there("sample-1", self.repository1, 1, 2) | ||
979 | 133 | yield self.assert_there("sample-2", self.repository1, 2) | ||
980 | 134 | yield self.assert_not_there("sample-3", self.repository1, 2) | ||
981 | 127 | 135 | ||
989 | 128 | file = open(os.path.join( | 136 | revision_path = os.path.join( |
990 | 129 | self.repository1.path, "series/old/metadata.yaml"), "rw+") | 137 | self.repository1.path, "series/old/revision") |
991 | 130 | data = yaml.load(file.read()) | 138 | with open(revision_path, "w") as f: |
992 | 131 | data["revision"] = 3 | 139 | f.write("3") |
986 | 132 | file.seek(0) | ||
987 | 133 | file.write(yaml.dump(data)) | ||
988 | 134 | file.close() | ||
993 | 135 | 140 | ||
994 | 136 | yield self.assert_there("sample", self.repository1, 3) | 141 | yield self.assert_there("sample", self.repository1, 3) |
995 | 142 | yield self.assert_not_there("sample-1", self.repository1, 3) | ||
996 | 137 | yield self.assert_there("sample-2", self.repository1, 2, 3) | 143 | yield self.assert_there("sample-2", self.repository1, 2, 3) |
997 | 144 | yield self.assert_there("sample-3", self.repository1, 3) | ||
998 | 138 | 145 | ||
999 | 139 | 146 | ||
1000 | 140 | class RemoteRepositoryTest(RepositoryTestBase): | 147 | class RemoteRepositoryTest(RepositoryTestBase): |
1001 | 141 | 148 | ||
1002 | === modified file 'juju/control/deploy.py' | |||
1003 | --- juju/control/deploy.py 2011-09-30 11:25:41 +0000 | |||
1004 | +++ juju/control/deploy.py 2011-10-05 18:13:26 +0000 | |||
1005 | @@ -95,7 +95,7 @@ | |||
1006 | 95 | service_options = parse_config_options(config_file, service_name) | 95 | service_options = parse_config_options(config_file, service_name) |
1007 | 96 | 96 | ||
1008 | 97 | charm = yield repo.find(charm_url) | 97 | charm = yield repo.find(charm_url) |
1010 | 98 | charm_id = str(charm_url.with_revision(charm.metadata.revision)) | 98 | charm_id = str(charm_url.with_revision(charm.get_revision())) |
1011 | 99 | 99 | ||
1012 | 100 | provider = environment.get_machine_provider() | 100 | provider = environment.get_machine_provider() |
1013 | 101 | placement_policy = provider.get_placement_policy() | 101 | placement_policy = provider.get_placement_policy() |
1014 | 102 | 102 | ||
1015 | === modified file 'juju/control/tests/test_upgrade_charm.py' | |||
1016 | --- juju/control/tests/test_upgrade_charm.py 2011-09-29 03:07:26 +0000 | |||
1017 | +++ juju/control/tests/test_upgrade_charm.py 2011-10-05 18:13:26 +0000 | |||
1018 | @@ -15,23 +15,23 @@ | |||
1019 | 15 | 15 | ||
1020 | 16 | class CharmUpgradeTestBase(object): | 16 | class CharmUpgradeTestBase(object): |
1021 | 17 | 17 | ||
1023 | 18 | def add_charm(self, metadata, repository_dir=None): | 18 | def add_charm(self, metadata, revision, repository_dir=None): |
1024 | 19 | if repository_dir is None: | 19 | if repository_dir is None: |
1025 | 20 | repository_dir = self.makeDir() | 20 | repository_dir = self.makeDir() |
1026 | 21 | series_dir = os.path.join(repository_dir, "series") | 21 | series_dir = os.path.join(repository_dir, "series") |
1027 | 22 | os.mkdir(series_dir) | 22 | os.mkdir(series_dir) |
1028 | 23 | charm_dir = os.path.join(series_dir, metadata["name"]) | 23 | charm_dir = os.path.join(series_dir, metadata["name"]) |
1029 | 24 | os.mkdir(charm_dir) | 24 | os.mkdir(charm_dir) |
1033 | 25 | fh = open(os.path.join(charm_dir, "metadata.yaml"), "w") | 25 | with open(os.path.join(charm_dir, "metadata.yaml"), "w") as f: |
1034 | 26 | fh.write(dump(metadata)) | 26 | f.write(dump(metadata)) |
1035 | 27 | fh.close() | 27 | with open(os.path.join(charm_dir, "revision"), "w") as f: |
1036 | 28 | f.write(str(revision)) | ||
1037 | 28 | return LocalCharmRepository(repository_dir) | 29 | return LocalCharmRepository(repository_dir) |
1038 | 29 | 30 | ||
1039 | 30 | def increment_charm(self, charm): | 31 | def increment_charm(self, charm): |
1040 | 31 | metadata = charm.metadata.get_serialization_data() | 32 | metadata = charm.metadata.get_serialization_data() |
1041 | 32 | metadata["name"] = "mysql" | 33 | metadata["name"] = "mysql" |
1044 | 33 | metadata["revision"] = 2 | 34 | repository = self.add_charm(metadata, charm.get_revision() + 1) |
1043 | 34 | repository = self.add_charm(metadata) | ||
1045 | 35 | return repository | 35 | return repository |
1046 | 36 | 36 | ||
1047 | 37 | 37 | ||
1048 | @@ -201,7 +201,7 @@ | |||
1049 | 201 | 201 | ||
1050 | 202 | metadata = self.charm.metadata.get_serialization_data() | 202 | metadata = self.charm.metadata.get_serialization_data() |
1051 | 203 | metadata["name"] = "mysql" | 203 | metadata["name"] = "mysql" |
1053 | 204 | repository = self.add_charm(metadata) | 204 | repository = self.add_charm(metadata, 1) |
1054 | 205 | main(["upgrade-charm", "--dry-run", | 205 | main(["upgrade-charm", "--dry-run", |
1055 | 206 | "--repository", repository.path, "mysql"]) | 206 | "--repository", repository.path, "mysql"]) |
1056 | 207 | yield finished | 207 | yield finished |
1057 | @@ -224,7 +224,7 @@ | |||
1058 | 224 | 224 | ||
1059 | 225 | metadata = self.charm.metadata.get_serialization_data() | 225 | metadata = self.charm.metadata.get_serialization_data() |
1060 | 226 | metadata["name"] = "mysql" | 226 | metadata["name"] = "mysql" |
1062 | 227 | repository = self.add_charm(metadata) | 227 | repository = self.add_charm(metadata, 1) |
1063 | 228 | main(["upgrade-charm", "--repository", repository.path, "mysql"]) | 228 | main(["upgrade-charm", "--repository", repository.path, "mysql"]) |
1064 | 229 | 229 | ||
1065 | 230 | yield finished | 230 | yield finished |
1066 | 231 | 231 | ||
1067 | === modified file 'juju/machine/tests/test_unit_deployment.py' | |||
1068 | --- juju/machine/tests/test_unit_deployment.py 2011-10-01 04:32:56 +0000 | |||
1069 | +++ juju/machine/tests/test_unit_deployment.py 2011-10-05 18:13:26 +0000 | |||
1070 | @@ -265,7 +265,7 @@ | |||
1071 | 265 | self.assertTrue(os.path.exists(charm_path)) | 265 | self.assertTrue(os.path.exists(charm_path)) |
1072 | 266 | charm = get_charm_from_path(charm_path) | 266 | charm = get_charm_from_path(charm_path) |
1073 | 267 | self.assertEqual( | 267 | self.assertEqual( |
1075 | 268 | charm.metadata.revision, self.charm.metadata.revision) | 268 | charm.get_revision(), self.charm.get_revision()) |
1076 | 269 | 269 | ||
1077 | 270 | def test_unpack_charm_exception_invalid_charm(self): | 270 | def test_unpack_charm_exception_invalid_charm(self): |
1078 | 271 | """ | 271 | """ |
1079 | 272 | 272 | ||
1080 | === modified file 'juju/state/charm.py' | |||
1081 | --- juju/state/charm.py 2011-09-28 23:54:04 +0000 | |||
1082 | +++ juju/state/charm.py 2011-10-05 18:13:26 +0000 | |||
1083 | @@ -73,7 +73,6 @@ | |||
1084 | 73 | 73 | ||
1085 | 74 | # Just a health check: | 74 | # Just a health check: |
1086 | 75 | assert self._metadata.name == self.name | 75 | assert self._metadata.name == self.name |
1087 | 76 | assert self._metadata.revision == self.revision | ||
1088 | 77 | 76 | ||
1089 | 78 | self._sha256 = charm_data["sha256"] | 77 | self._sha256 = charm_data["sha256"] |
1090 | 79 | 78 | ||
1091 | 80 | 79 | ||
1092 | === modified file 'juju/state/tests/test_charm.py' | |||
1093 | --- juju/state/tests/test_charm.py 2011-09-28 00:36:30 +0000 | |||
1094 | +++ juju/state/tests/test_charm.py 2011-10-05 18:13:26 +0000 | |||
1095 | @@ -79,7 +79,6 @@ | |||
1096 | 79 | "local:series/dummy-1") | 79 | "local:series/dummy-1") |
1097 | 80 | metadata = yield charm_state.get_metadata() | 80 | metadata = yield charm_state.get_metadata() |
1098 | 81 | self.assertEquals(metadata.name, "dummy") | 81 | self.assertEquals(metadata.name, "dummy") |
1099 | 82 | self.assertEquals(metadata.revision, 1) | ||
1100 | 83 | 82 | ||
1101 | 84 | @inlineCallbacks | 83 | @inlineCallbacks |
1102 | 85 | def test_charm_state_config_options(self): | 84 | def test_charm_state_config_options(self): |
1103 | 86 | 85 | ||
1104 | === modified file 'juju/unit/tests/test_charm.py' | |||
1105 | --- juju/unit/tests/test_charm.py 2011-09-28 23:04:39 +0000 | |||
1106 | +++ juju/unit/tests/test_charm.py 2011-10-05 18:13:26 +0000 | |||
1107 | @@ -84,7 +84,7 @@ | |||
1108 | 84 | 84 | ||
1109 | 85 | self.assertTrue(os.path.exists(charm_path)) | 85 | self.assertTrue(os.path.exists(charm_path)) |
1110 | 86 | bundle = CharmBundle(charm_path) | 86 | bundle = CharmBundle(charm_path) |
1112 | 87 | self.assertEquals(bundle.metadata.revision, charm.metadata.revision) | 87 | self.assertEquals(bundle.get_revision(), charm.get_revision()) |
1113 | 88 | 88 | ||
1114 | 89 | self.assertEqual(checksum, bundle.get_sha256()) | 89 | self.assertEqual(checksum, bundle.get_sha256()) |
1115 | 90 | 90 |
This is looking like a good start, but there is one important
conceptual issue we should probably address while we have time.
[1]
+ metadata = MetaData( get_revision_ content= self._get_ revision_ content) parse(zf. read("metadata. yaml"))
+ metadata.
+ if metadata.revision is None:
+ raise CharmError(path, "has no revision")
Hmmm.. this is feeling a bit like a hack. Right now the concepts within
the charm are well separated in the API as well.. we have the config
with the contents of config.yaml and the metadata with the
contents of metadata.yaml. We're changing the location of the
revision information so that we separate logically the bits that are
human-editable from the bits that are automatically changed. If this
is a good idea, we should do this across the board, and take the
revision out of the metadata as well within the code base.
So, let's remove metadata.revision, and introduce something like a obsolete_ revision which is only accessed from the charm obsolete_ revision revision( ) instead, or to something contextually equivalent.
metadata.
implementation itself. Then, inside the charm implementation let's
introduce a method called get_revision() which either picks the
revision from the revision file or from the metadata.
if the former is missing. Every other place that looks at
metadata.revision today should be changed to look at
charm.get_
The storage and API in juju/state/charm.py will also have to change
accordingly, for instance.
This is certainly what we would have done if we started with the
revision separated in the first place, for instance.
Perhaps this will turn out to be too painful, but if we cannot do this
on time, I'd rather not change the revision location, otherwise we'll
be introducing a half-baked feature that looks like one thing in some
places and like something else in others, and forever have that hackish
taste when dealing with it.
[2]
+ def set_revision(self, revision): path.join( self.path, "revision"), "w") as f: str(revision) )
+ with open(os.
+ f.write(
That's nice, and it shows the above point. set_revision is in the
charm, not in the metadata. That's where we need get_revision to
be as well.
[3]
+ try: _get_revision_ content( ))
+ return int(self.
+ except (ValueError, TypeError):
+ pass
If the file exists, and its content is invalid, we need to raise a
real error about the problem than pretending it doesn't exist.
[4]
+ if "revision" in self._data:
+ suffix = (" from %r" % path) if path else ""
+ log.warning(
+ "metadata file schema has changed: revision should be "
+ "removed" + suffix)
I suggest only logging the warning if the file path is known, as I
believe this will mean the user has a better chance of being able to
fix the problem rather than being just a consumer. Is that the case?
If so, I suggest something like this:
if "revision" in self._data and path:
log.warning(
"%s: metadata.yaml: revision field is obsolete. "
"Move it to the 'revision' file." % path)