Merge lp:~fwereade/pyjuju/cobbler-zk-connect-error-messages into lp:~fwereade/pyjuju/cobbler-zk-connect
- cobbler-zk-connect-error-messages
- Merge into cobbler-zk-connect
Status: | Merged | ||||
---|---|---|---|---|---|
Approved by: | William Reade | ||||
Approved revision: | 339 | ||||
Merged at revision: | 339 | ||||
Proposed branch: | lp:~fwereade/pyjuju/cobbler-zk-connect-error-messages | ||||
Merge into: | lp:~fwereade/pyjuju/cobbler-zk-connect | ||||
Diff against target: |
1156 lines (+192/-174) 43 files modified
ensemble/agents/provision.py (+4/-4) ensemble/agents/tests/test_provision.py (+3/-3) ensemble/agents/tests/test_unit.py (+4/-4) ensemble/control/status.py (+3/-3) ensemble/control/tests/test_status.py (+0/-2) ensemble/errors.py (+7/-19) ensemble/formula/bundle.py (+3/-3) ensemble/formula/provider.py (+1/-1) ensemble/formula/tests/test_bundle.py (+8/-4) ensemble/formula/tests/test_config.py (+6/-4) ensemble/formula/tests/test_repository.py (+4/-4) ensemble/hooks/tests/test_arguments.py (+2/-2) ensemble/hooks/tests/test_cli.py (+5/-2) ensemble/hooks/tests/test_executor.py (+1/-1) ensemble/hooks/tests/test_invoker.py (+1/-1) ensemble/lib/tests/test_schema.py (+1/-1) ensemble/machine/tests/test_unit_deployment.py (+6/-4) ensemble/machine/unit.py (+1/-1) ensemble/providers/common/connect.py (+2/-2) ensemble/providers/common/findzookeepers.py (+1/-1) ensemble/providers/common/tests/test_connect.py (+5/-7) ensemble/providers/common/tests/test_findzookeepers.py (+27/-2) ensemble/providers/common/tests/test_utils.py (+2/-5) ensemble/providers/common/utils.py (+3/-1) ensemble/providers/ec2/__init__.py (+3/-2) ensemble/providers/ec2/launch.py (+2/-3) ensemble/providers/ec2/securitygroup.py (+6/-7) ensemble/providers/ec2/tests/common.py (+0/-10) ensemble/providers/ec2/tests/test_connect.py (+5/-7) ensemble/providers/ec2/tests/test_getmachines.py (+2/-3) ensemble/providers/ec2/tests/test_launch.py (+3/-4) ensemble/providers/ec2/tests/test_provider.py (+4/-1) ensemble/providers/ec2/tests/test_securitygroup.py (+6/-6) ensemble/providers/orchestra/cobbler.py (+5/-5) ensemble/providers/orchestra/tests/test_cobbler.py (+14/-9) ensemble/providers/orchestra/tests/test_connect.py (+4/-4) ensemble/providers/orchestra/tests/test_launch.py (+14/-6) ensemble/providers/orchestra/tests/test_provider.py (+4/-2) ensemble/state/service.py (+2/-1) ensemble/state/tests/test_security.py (+1/-1) ensemble/state/tests/test_service.py (+9/-5) ensemble/tests/test_errors.py (+6/-16) ensemble/unit/tests/test_formula.py (+2/-1) |
||||
To merge this branch: | bzr merge lp:~fwereade/pyjuju/cobbler-zk-connect-error-messages | ||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Jim Baker (community) | Approve | ||
Gustavo Niemeyer (community) | Approve | ||
Review via email: mp+72033@code.launchpad.net |
Commit message
Description of the change
Fixed errors as requested in https:/
Also made many assertIn error message checks into assertEquals; improved some error messages; reworked ProviderInterac
- 333. By William Reade
-
merge parent
- 334. By William Reade
-
merge parent
- 335. By William Reade
-
merge parent
- 336. By William Reade
-
merge parent
William Reade (fwereade) wrote : | # |
[error messages]
The only solution I can see to all these issues is to eliminate all error message prefixes, and to write out the full error message for every exception type at every call site... for example, I thought you didn't like the "Colon-joined sentence fragment: Style of error message", but you're asking me to reinstate it in [2] and [3]. WRT [1], people were already using the "simple" form incorrectly, and the same fix applies.
[docstrings]
Reverting; opened lp:831805
William Reade (fwereade) wrote : | # |
> [error messages]
>
> "Colon-joined sentence fragment: Style of error message", but you're asking me to
> reinstate it in [2] and [3].
On further inspection, FormulaError(
- 337. By William Reade
-
merge parent
- 338. By William Reade
-
fix ProviderInterac
tionErrors per review - 339. By William Reade
-
merge parent
Gustavo Niemeyer (niemeyer) wrote : | # |
> On further inspection, FormulaError(
> really does appear to be the standard, because it results in a capitalised
> error message without weird interspersed caps, and I'll be leaving them as-is.
> If I've misunderstood I'll do a separate cleanup branch.
Sounds good then. Thanks for checking rather than taking the crackful
comment blindly. :-)
Jim Baker (jimbaker) wrote : | # |
+1, overall this looks good. Moving to assertEquals is definitely an improvement as well. There are a number of spots where you should consider using %r instead of %s where file names and potentially other strings might be Unicode. There is one case that certainly should be done that way:
def __str__(self):
- return "Error processing '%s': %s." % (self.path, self.message)
+ return "Error processing '%s': %s" % (self.path, self.message)
Jim Baker (jimbaker) wrote : | # |
Interestingly I cannot mark the branch as being approved now that we have hit our quorum of reviewers. Is this because it's stacked? Anyway, someone should take this action if at all possible!
William Reade (fwereade) wrote : | # |
> Interestingly I cannot mark the branch as being approved now that we have hit
> our quorum of reviewers. Is this because it's stacked? Anyway, someone should
> take this action if at all possible!
Feels funny approving my own branch :). I'll make your changes and get it in shortly. Thanks for the review :).
- 340. By William Reade
-
merge parent
- 341. By William Reade
-
fixes per review
Preview Diff
1 | === modified file 'ensemble/agents/provision.py' | |||
2 | --- ensemble/agents/provision.py 2011-08-12 19:28:37 +0000 | |||
3 | +++ ensemble/agents/provision.py 2011-08-26 13:09:07 +0000 | |||
4 | @@ -4,7 +4,7 @@ | |||
5 | 4 | from zookeeper import NoNodeException | 4 | from zookeeper import NoNodeException |
6 | 5 | 5 | ||
7 | 6 | from ensemble.environment.config import EnvironmentsConfig | 6 | from ensemble.environment.config import EnvironmentsConfig |
9 | 7 | from ensemble.errors import MachinesNotFound, ProviderInteractionError | 7 | from ensemble.errors import MachinesNotFound, ProviderError |
10 | 8 | from ensemble.lib.twistutils import concurrent_execution_guard | 8 | from ensemble.lib.twistutils import concurrent_execution_guard |
11 | 9 | from ensemble.state.errors import ( | 9 | from ensemble.state.errors import ( |
12 | 10 | MachineStateNotFound, ServiceStateNotFound, ServiceUnitStateNotFound, | 10 | MachineStateNotFound, ServiceStateNotFound, ServiceUnitStateNotFound, |
13 | @@ -178,7 +178,7 @@ | |||
14 | 178 | # map of instance_id -> machine | 178 | # map of instance_id -> machine |
15 | 179 | try: | 179 | try: |
16 | 180 | provider_machines = yield self.provider.get_machines() | 180 | provider_machines = yield self.provider.get_machines() |
18 | 181 | except ProviderInteractionError: | 181 | except ProviderError: |
19 | 182 | log.exception("Cannot get machine list") | 182 | log.exception("Cannot get machine list") |
20 | 183 | return | 183 | return |
21 | 184 | 184 | ||
22 | @@ -192,7 +192,7 @@ | |||
23 | 192 | machine_state_id, provider_machines) | 192 | machine_state_id, provider_machines) |
24 | 193 | except (StateChanged, | 193 | except (StateChanged, |
25 | 194 | MachineStateNotFound, | 194 | MachineStateNotFound, |
27 | 195 | ProviderInteractionError): | 195 | ProviderError): |
28 | 196 | log.exception("Cannot process machine %s", machine_state_id) | 196 | log.exception("Cannot process machine %s", machine_state_id) |
29 | 197 | continue | 197 | continue |
30 | 198 | instance_ids.append(instance_id) | 198 | instance_ids.append(instance_id) |
31 | @@ -204,7 +204,7 @@ | |||
32 | 204 | machine = provider_machines[instance_id] | 204 | machine = provider_machines[instance_id] |
33 | 205 | try: | 205 | try: |
34 | 206 | yield self.provider.shutdown_machine(machine) | 206 | yield self.provider.shutdown_machine(machine) |
36 | 207 | except ProviderInteractionError: | 207 | except ProviderError: |
37 | 208 | log.exception("Cannot shutdown machine %s", instance_id) | 208 | log.exception("Cannot shutdown machine %s", instance_id) |
38 | 209 | continue | 209 | continue |
39 | 210 | 210 | ||
40 | 211 | 211 | ||
41 | === modified file 'ensemble/agents/tests/test_provision.py' | |||
42 | --- ensemble/agents/tests/test_provision.py 2011-08-11 05:59:30 +0000 | |||
43 | +++ ensemble/agents/tests/test_provision.py 2011-08-26 13:09:07 +0000 | |||
44 | @@ -313,7 +313,7 @@ | |||
45 | 313 | 313 | ||
46 | 314 | mock_provider = self.mocker.patch(self.agent.provider) | 314 | mock_provider = self.mocker.patch(self.agent.provider) |
47 | 315 | mock_provider.start_machine({"machine-id": 0}) | 315 | mock_provider.start_machine({"machine-id": 0}) |
49 | 316 | self.mocker.result(fail(ProviderInteractionError(OSError("Bad")))) | 316 | self.mocker.result(fail(ProviderInteractionError())) |
50 | 317 | 317 | ||
51 | 318 | mock_provider.start_machine({"machine-id": 1}) | 318 | mock_provider.start_machine({"machine-id": 1}) |
52 | 319 | self.mocker.passthrough() | 319 | self.mocker.passthrough() |
53 | @@ -339,7 +339,7 @@ | |||
54 | 339 | mock_provider = self.mocker.patch(self.agent.provider) | 339 | mock_provider = self.mocker.patch(self.agent.provider) |
55 | 340 | 340 | ||
56 | 341 | mock_provider.shutdown_machine(MATCH_MACHINE) | 341 | mock_provider.shutdown_machine(MATCH_MACHINE) |
58 | 342 | self.mocker.result(fail(ProviderInteractionError("Bad"))) | 342 | self.mocker.result(fail(ProviderInteractionError())) |
59 | 343 | 343 | ||
60 | 344 | mock_provider.shutdown_machine(MATCH_MACHINE) | 344 | mock_provider.shutdown_machine(MATCH_MACHINE) |
61 | 345 | self.mocker.passthrough() | 345 | self.mocker.passthrough() |
62 | @@ -368,7 +368,7 @@ | |||
63 | 368 | 368 | ||
64 | 369 | mock_provider = self.mocker.patch(self.agent.provider) | 369 | mock_provider = self.mocker.patch(self.agent.provider) |
65 | 370 | mock_provider.get_machines() | 370 | mock_provider.get_machines() |
67 | 371 | self.mocker.result(fail(ProviderInteractionError("Bad"))) | 371 | self.mocker.result(fail(ProviderInteractionError())) |
68 | 372 | 372 | ||
69 | 373 | mock_provider.get_machines() | 373 | mock_provider.get_machines() |
70 | 374 | self.mocker.passthrough() | 374 | self.mocker.passthrough() |
71 | 375 | 375 | ||
72 | === modified file 'ensemble/agents/tests/test_unit.py' | |||
73 | --- ensemble/agents/tests/test_unit.py 2011-07-29 14:42:04 +0000 | |||
74 | +++ ensemble/agents/tests/test_unit.py 2011-08-26 13:09:07 +0000 | |||
75 | @@ -118,10 +118,10 @@ | |||
76 | 118 | e = self.assertRaises(EnsembleError, | 118 | e = self.assertRaises(EnsembleError, |
77 | 119 | self.agent.configure, | 119 | self.agent.configure, |
78 | 120 | options) | 120 | options) |
83 | 121 | self.assertIn( | 121 | self.assertEquals( |
84 | 122 | ("--unit-name must be provided in the command line," | 122 | str(e), |
85 | 123 | " or $ENSEMBLE_UNIT_NAME in the environment"), | 123 | "--unit-name must be provided in the command line, or " |
86 | 124 | str(e)) | 124 | "$ENSEMBLE_UNIT_NAME in the environment") |
87 | 125 | 125 | ||
88 | 126 | def test_agent_unit_name_cli_extraction(self): | 126 | def test_agent_unit_name_cli_extraction(self): |
89 | 127 | """The unit agent can parse its unit-name from the cli. | 127 | """The unit agent can parse its unit-name from the cli. |
90 | 128 | 128 | ||
91 | === modified file 'ensemble/control/status.py' | |||
92 | --- ensemble/control/status.py 2011-07-20 23:35:48 +0000 | |||
93 | +++ ensemble/control/status.py 2011-08-26 13:09:07 +0000 | |||
94 | @@ -7,7 +7,7 @@ | |||
95 | 7 | from twisted.internet.defer import inlineCallbacks, returnValue | 7 | from twisted.internet.defer import inlineCallbacks, returnValue |
96 | 8 | import yaml | 8 | import yaml |
97 | 9 | 9 | ||
99 | 10 | from ensemble.errors import ProviderInteractionError | 10 | from ensemble.errors import ProviderError |
100 | 11 | from ensemble.environment.errors import EnvironmentsConfigError | 11 | from ensemble.environment.errors import EnvironmentsConfigError |
101 | 12 | from ensemble.state.errors import UnitRelationStateNotFound | 12 | from ensemble.state.errors import UnitRelationStateNotFound |
102 | 13 | from ensemble.state.formula import FormulaStateManager | 13 | from ensemble.state.formula import FormulaStateManager |
103 | @@ -264,10 +264,10 @@ | |||
104 | 264 | try: | 264 | try: |
105 | 265 | pm = yield machine_provider.get_machine(instance_id) | 265 | pm = yield machine_provider.get_machine(instance_id) |
106 | 266 | m["dns-name"] = pm.dns_name | 266 | m["dns-name"] = pm.dns_name |
108 | 267 | except ProviderInteractionError: | 267 | except ProviderError: |
109 | 268 | # The provider doesn't have machine information | 268 | # The provider doesn't have machine information |
110 | 269 | log.error("Machine provider information missing: machine %s" % ( | 269 | log.error("Machine provider information missing: machine %s" % ( |
112 | 270 | machine_state.id)) | 270 | machine_state.id)) |
113 | 271 | 271 | ||
114 | 272 | machine_data[machine_state.id] = m | 272 | machine_data[machine_state.id] = m |
115 | 273 | 273 | ||
116 | 274 | 274 | ||
117 | === modified file 'ensemble/control/tests/test_status.py' | |||
118 | --- ensemble/control/tests/test_status.py 2011-08-08 21:51:54 +0000 | |||
119 | +++ ensemble/control/tests/test_status.py 2011-08-26 13:09:07 +0000 | |||
120 | @@ -458,7 +458,6 @@ | |||
121 | 458 | self.assertEqual(state["machines"][7]["instance-id"], | 458 | self.assertEqual(state["machines"][7]["instance-id"], |
122 | 459 | "pending") | 459 | "pending") |
123 | 460 | 460 | ||
124 | 461 | |||
125 | 462 | @inlineCallbacks | 461 | @inlineCallbacks |
126 | 463 | def test_render_yaml(self): | 462 | def test_render_yaml(self): |
127 | 464 | yield self.build_topology() | 463 | yield self.build_topology() |
128 | @@ -610,7 +609,6 @@ | |||
129 | 610 | 609 | ||
130 | 611 | self.assertIn("namespace:dummy-1", result) | 610 | self.assertIn("namespace:dummy-1", result) |
131 | 612 | 611 | ||
132 | 613 | |||
133 | 614 | def test_render_dot_bad_clustering(self): | 612 | def test_render_dot_bad_clustering(self): |
134 | 615 | """Test around Bug #792448. | 613 | """Test around Bug #792448. |
135 | 616 | 614 | ||
136 | 617 | 615 | ||
137 | === modified file 'ensemble/errors.py' | |||
138 | --- ensemble/errors.py 2011-08-17 17:26:19 +0000 | |||
139 | +++ ensemble/errors.py 2011-08-26 13:09:07 +0000 | |||
140 | @@ -48,7 +48,7 @@ | |||
141 | 48 | self.path = path | 48 | self.path = path |
142 | 49 | 49 | ||
143 | 50 | def __str__(self): | 50 | def __str__(self): |
145 | 51 | return "File was not found: %s" % (self.path,) | 51 | return "File was not found: %r" % (self.path,) |
146 | 52 | 52 | ||
147 | 53 | 53 | ||
148 | 54 | class FormulaError(EnsembleError): | 54 | class FormulaError(EnsembleError): |
149 | @@ -59,7 +59,7 @@ | |||
150 | 59 | self.message = message | 59 | self.message = message |
151 | 60 | 60 | ||
152 | 61 | def __str__(self): | 61 | def __str__(self): |
154 | 62 | return "Error processing '%s': %s." % (self.path, self.message) | 62 | return "Error processing %r: %s" % (self.path, self.message) |
155 | 63 | 63 | ||
156 | 64 | 64 | ||
157 | 65 | class FormulaInvocationError(EnsembleError): | 65 | class FormulaInvocationError(EnsembleError): |
158 | @@ -70,8 +70,8 @@ | |||
159 | 70 | self.exit_code = exit_code | 70 | self.exit_code = exit_code |
160 | 71 | 71 | ||
161 | 72 | def __str__(self): | 72 | def __str__(self): |
164 | 73 | return "Error processing '%s': exit code %s." % (self.path, | 73 | return "Error processing %r: exit code %s." % ( |
165 | 74 | self.exit_code) | 74 | self.path, self.exit_code) |
166 | 75 | 75 | ||
167 | 76 | 76 | ||
168 | 77 | class FileAlreadyExists(EnsembleError): | 77 | class FileAlreadyExists(EnsembleError): |
169 | @@ -84,7 +84,7 @@ | |||
170 | 84 | self.path = path | 84 | self.path = path |
171 | 85 | 85 | ||
172 | 86 | def __str__(self): | 86 | def __str__(self): |
174 | 87 | return "File already exists, won't overwrite: %s" % (self.path,) | 87 | return "File already exists, won't overwrite: %r" % (self.path,) |
175 | 88 | 88 | ||
176 | 89 | 89 | ||
177 | 90 | class InvalidEnsembleHeaderValue(EnsembleError): | 90 | class InvalidEnsembleHeaderValue(EnsembleError): |
178 | @@ -101,7 +101,7 @@ | |||
179 | 101 | 101 | ||
180 | 102 | def __str__(self): | 102 | def __str__(self): |
181 | 103 | return ("Expected a YAML file with an 'ensemble: %s' " | 103 | return ("Expected a YAML file with an 'ensemble: %s' " |
183 | 104 | "header, found 'ensemble: %s': %s" | 104 | "header, found 'ensemble: %s': %r" |
184 | 105 | % (self.expected, self.found, self.path)) | 105 | % (self.expected, self.found, self.path)) |
185 | 106 | 106 | ||
186 | 107 | 107 | ||
187 | @@ -130,12 +130,6 @@ | |||
188 | 130 | class EnvironmentPending(NoConnection): | 130 | class EnvironmentPending(NoConnection): |
189 | 131 | """Raised when the ensemble environment is not accessible.""" | 131 | """Raised when the ensemble environment is not accessible.""" |
190 | 132 | 132 | ||
191 | 133 | def __init__(self, info="no details available"): | ||
192 | 134 | self._info = info | ||
193 | 135 | |||
194 | 136 | def __str__(self): | ||
195 | 137 | return "Ensemble environment is not accessible: %s" % self._info | ||
196 | 138 | |||
197 | 139 | 133 | ||
198 | 140 | class ProviderError(EnsembleError): | 134 | class ProviderError(EnsembleError): |
199 | 141 | """Raised when an exception occurs in a provider.""" | 135 | """Raised when an exception occurs in a provider.""" |
200 | @@ -154,13 +148,7 @@ | |||
201 | 154 | 148 | ||
202 | 155 | 149 | ||
203 | 156 | class ProviderInteractionError(ProviderError): | 150 | class ProviderInteractionError(ProviderError): |
211 | 157 | 151 | """Raised when an unexpected error occurs interacting with a provider""" | |
205 | 158 | def __init__(self, error): | ||
206 | 159 | self.error = error | ||
207 | 160 | |||
208 | 161 | def __str__(self): | ||
209 | 162 | return "ProviderError: Interaction with machine provider failed: %r" \ | ||
210 | 163 | % self.error | ||
212 | 164 | 152 | ||
213 | 165 | 153 | ||
214 | 166 | class CannotTerminateMachine(EnsembleError): | 154 | class CannotTerminateMachine(EnsembleError): |
215 | 167 | 155 | ||
216 | === modified file 'ensemble/formula/bundle.py' | |||
217 | --- ensemble/formula/bundle.py 2011-06-05 20:19:18 +0000 | |||
218 | +++ ensemble/formula/bundle.py 2011-08-26 13:09:07 +0000 | |||
219 | @@ -18,13 +18,13 @@ | |||
220 | 18 | try: | 18 | try: |
221 | 19 | zf = ZipFile(path, 'r') | 19 | zf = ZipFile(path, 'r') |
222 | 20 | except BadZipfile, exc: | 20 | except BadZipfile, exc: |
224 | 21 | raise FormulaError(path, "Must be a ZIP-file (%s)." % str(exc)) | 21 | raise FormulaError(path, "must be a zip file (%s)" % exc) |
225 | 22 | 22 | ||
226 | 23 | metadata = MetaData() | 23 | metadata = MetaData() |
227 | 24 | if "metadata.yaml" not in zf.namelist(): | 24 | if "metadata.yaml" not in zf.namelist(): |
228 | 25 | raise FormulaError(path, | 25 | raise FormulaError(path, |
231 | 26 | "Archive does not contain required file: " | 26 | "archive does not contain required file " |
232 | 27 | "\"metadata.yaml\".") | 27 | "\"metadata.yaml\"") |
233 | 28 | 28 | ||
234 | 29 | content = zf.read("metadata.yaml") | 29 | content = zf.read("metadata.yaml") |
235 | 30 | metadata.parse(content) | 30 | metadata.parse(content) |
236 | 31 | 31 | ||
237 | === modified file 'ensemble/formula/provider.py' | |||
238 | --- ensemble/formula/provider.py 2010-11-02 16:15:23 +0000 | |||
239 | +++ ensemble/formula/provider.py 2011-08-26 13:09:07 +0000 | |||
240 | @@ -25,5 +25,5 @@ | |||
241 | 25 | from .directory import FormulaDirectory | 25 | from .directory import FormulaDirectory |
242 | 26 | return FormulaDirectory(specification) | 26 | return FormulaDirectory(specification) |
243 | 27 | 27 | ||
245 | 28 | raise FormulaError(specification, "Unable to process %s into a formula" % ( | 28 | raise FormulaError(specification, "unable to process %s into a formula" % ( |
246 | 29 | specification)) | 29 | specification)) |
247 | 30 | 30 | ||
248 | === modified file 'ensemble/formula/tests/test_bundle.py' | |||
249 | --- ensemble/formula/tests/test_bundle.py 2011-05-06 04:45:17 +0000 | |||
250 | +++ ensemble/formula/tests/test_bundle.py 2011-08-26 13:09:07 +0000 | |||
251 | @@ -38,8 +38,10 @@ | |||
252 | 38 | try: | 38 | try: |
253 | 39 | FormulaBundle(filename) | 39 | FormulaBundle(filename) |
254 | 40 | except FormulaError, exc: | 40 | except FormulaError, exc: |
257 | 41 | self.assertIn(filename, str(exc)) | 41 | self.assertEquals( |
258 | 42 | self.assertIn("zip", str(exc).lower()) | 42 | str(exc), |
259 | 43 | ("Error processing %r: " % filename) + | ||
260 | 44 | "must be a zip file (File is not a zip file)") | ||
261 | 43 | return | 45 | return |
262 | 44 | 46 | ||
263 | 45 | self.fail("Expected formula error.") | 47 | self.fail("Expected formula error.") |
264 | @@ -53,8 +55,10 @@ | |||
265 | 53 | try: | 55 | try: |
266 | 54 | FormulaBundle(filename) | 56 | FormulaBundle(filename) |
267 | 55 | except FormulaError, exc: | 57 | except FormulaError, exc: |
270 | 56 | self.assertIn(filename, str(exc)) | 58 | self.assertEquals( |
271 | 57 | self.assertIn("metadata.yaml", str(exc).lower()) | 59 | str(exc), |
272 | 60 | ("Error processing %r: " % filename) + | ||
273 | 61 | "archive does not contain required file \"metadata.yaml\"") | ||
274 | 58 | return | 62 | return |
275 | 59 | 63 | ||
276 | 60 | self.fail("Expected formula error.") | 64 | self.fail("Expected formula error.") |
277 | 61 | 65 | ||
278 | === modified file 'ensemble/formula/tests/test_config.py' | |||
279 | --- ensemble/formula/tests/test_config.py 2011-08-24 00:18:27 +0000 | |||
280 | +++ ensemble/formula/tests/test_config.py 2011-08-26 13:09:07 +0000 | |||
281 | @@ -102,7 +102,7 @@ | |||
282 | 102 | 102 | ||
283 | 103 | error = self.assertRaises(ServiceConfigError, | 103 | error = self.assertRaises(ServiceConfigError, |
284 | 104 | self.config.validate, dict(username="1234")) | 104 | self.config.validate, dict(username="1234")) |
286 | 105 | self.assertIn(str(error), "Invalid value for username: 1234") | 105 | self.assertEquals(str(error), "Invalid value for username: 1234") |
287 | 106 | 106 | ||
288 | 107 | data = self.config.validate(dict(username="12345678")) | 107 | data = self.config.validate(dict(username="12345678")) |
289 | 108 | self.assertEqual(data, {"username": "12345678", "title": "My Title"}) | 108 | self.assertEqual(data, {"username": "12345678", "title": "My Title"}) |
290 | @@ -114,15 +114,17 @@ | |||
291 | 114 | error = self.assertRaises(ServiceConfigError, | 114 | error = self.assertRaises(ServiceConfigError, |
292 | 115 | self.config.parse, | 115 | self.config.parse, |
293 | 116 | local_configuration) | 116 | local_configuration) |
296 | 117 | self.assertIn("options.fails.validator: required value not found", | 117 | self.assertEquals( |
297 | 118 | str(error)) | 118 | str(error), |
298 | 119 | "Invalid options specification: options.fails.validator: " | ||
299 | 120 | "required value not found") | ||
300 | 119 | 121 | ||
301 | 120 | def test_validate_types(self): | 122 | def test_validate_types(self): |
302 | 121 | self.config.parse(sample_configuration) | 123 | self.config.parse(sample_configuration) |
303 | 122 | 124 | ||
304 | 123 | error = self.assertRaises(ServiceConfigError, | 125 | error = self.assertRaises(ServiceConfigError, |
305 | 124 | self.config.validate, {"skill-level": "NaN"}) | 126 | self.config.validate, {"skill-level": "NaN"}) |
307 | 125 | self.assertIn(str(error), "Invalid value for skill-level: NaN") | 127 | self.assertEquals(str(error), "Invalid value for skill-level: NaN") |
308 | 126 | 128 | ||
309 | 127 | data = self.config.validate({"skill-level": "9001"}) | 129 | data = self.config.validate({"skill-level": "9001"}) |
310 | 128 | # its over 9000! | 130 | # its over 9000! |
311 | 129 | 131 | ||
312 | === modified file 'ensemble/formula/tests/test_repository.py' | |||
313 | --- ensemble/formula/tests/test_repository.py 2011-07-15 12:41:01 +0000 | |||
314 | +++ ensemble/formula/tests/test_repository.py 2011-08-26 13:09:07 +0000 | |||
315 | @@ -85,11 +85,11 @@ | |||
316 | 85 | self.assertTrue(formula.metadata) | 85 | self.assertTrue(formula.metadata) |
317 | 86 | 86 | ||
318 | 87 | def test_find_formula_by_name_fails(self): | 87 | def test_find_formula_by_name_fails(self): |
320 | 88 | exc = self.assertRaises( | 88 | error = self.assertRaises( |
321 | 89 | FormulaNotFound, self.repository1.find, "missing") | 89 | FormulaNotFound, self.repository1.find, "missing") |
325 | 90 | self.assertIn("missing", str(exc)) | 90 | self.assertEquals(str(error), |
326 | 91 | self.assertIn(self.unbundled_repo_path, str(exc), | 91 | "Formula 'missing' not found in repository %s" |
327 | 92 | "Repository path not found in the error message.") | 92 | % self.unbundled_repo_path) |
328 | 93 | 93 | ||
329 | 94 | def test_find_formula_with_multiple_versions_returns_latest(self): | 94 | def test_find_formula_with_multiple_versions_returns_latest(self): |
330 | 95 | # Copy the repository out of the codebase, so that we can hack it. | 95 | # Copy the repository out of the codebase, so that we can hack it. |
331 | 96 | 96 | ||
332 | === modified file 'ensemble/hooks/tests/test_arguments.py' | |||
333 | --- ensemble/hooks/tests/test_arguments.py 2011-06-11 02:37:35 +0000 | |||
334 | +++ ensemble/hooks/tests/test_arguments.py 2011-08-26 13:09:07 +0000 | |||
335 | @@ -52,8 +52,8 @@ | |||
336 | 52 | err = self.capture_stream("stderr") | 52 | err = self.capture_stream("stderr") |
337 | 53 | os.environ.pop("ENSEMBLE_AGENT_SOCKET", None) | 53 | os.environ.pop("ENSEMBLE_AGENT_SOCKET", None) |
338 | 54 | error = self.failUnlessRaises(SystemExit, cli.parse_args) | 54 | error = self.failUnlessRaises(SystemExit, cli.parse_args) |
341 | 55 | self.assertEquals("No ENSEMBLE_AGENT_SOCKET/-s option found", | 55 | self.assertEquals(str(error), |
342 | 56 | str(error)) | 56 | "No ENSEMBLE_AGENT_SOCKET/-s option found") |
343 | 57 | self.assertIn("No ENSEMBLE_AGENT_SOCKET/-s option found", | 57 | self.assertIn("No ENSEMBLE_AGENT_SOCKET/-s option found", |
344 | 58 | err.getvalue()) | 58 | err.getvalue()) |
345 | 59 | 59 | ||
346 | 60 | 60 | ||
347 | === modified file 'ensemble/hooks/tests/test_cli.py' | |||
348 | --- ensemble/hooks/tests/test_cli.py 2011-06-29 09:02:33 +0000 | |||
349 | +++ ensemble/hooks/tests/test_cli.py 2011-08-26 13:09:07 +0000 | |||
350 | @@ -335,12 +335,15 @@ | |||
351 | 335 | # and check an error condition | 335 | # and check an error condition |
352 | 336 | options = ["content=@missing"] | 336 | options = ["content=@missing"] |
353 | 337 | error = self.assertRaises(EnsembleError, parse_keyvalue_pairs, options) | 337 | error = self.assertRaises(EnsembleError, parse_keyvalue_pairs, options) |
355 | 338 | self.assertIn("No such file", str(error)) | 338 | self.assertEquals( |
356 | 339 | str(error), | ||
357 | 340 | "No such file or directory: missing (argument:content)") | ||
358 | 339 | 341 | ||
359 | 340 | # and check when fed non-kvpairs the error makes sense | 342 | # and check when fed non-kvpairs the error makes sense |
360 | 341 | options = ["foobar"] | 343 | options = ["foobar"] |
361 | 342 | error = self.assertRaises(EnsembleError, parse_keyvalue_pairs, options) | 344 | error = self.assertRaises(EnsembleError, parse_keyvalue_pairs, options) |
363 | 343 | self.assertIn("Expected `option=value`. Found `foobar`", str(error)) | 345 | self.assertEquals( |
364 | 346 | str(error), "Expected `option=value`. Found `foobar`") | ||
365 | 344 | 347 | ||
366 | 345 | def test_parse_port_protocol(self): | 348 | def test_parse_port_protocol(self): |
367 | 346 | self.assertEqual(parse_port_protocol("80"), (80, "tcp")) | 349 | self.assertEqual(parse_port_protocol("80"), (80, "tcp")) |
368 | 347 | 350 | ||
369 | === modified file 'ensemble/hooks/tests/test_executor.py' | |||
370 | --- ensemble/hooks/tests/test_executor.py 2011-07-06 18:00:07 +0000 | |||
371 | +++ ensemble/hooks/tests/test_executor.py 2011-08-26 13:09:07 +0000 | |||
372 | @@ -326,7 +326,7 @@ | |||
373 | 326 | error = yield self.assertFailure( | 326 | error = yield self.assertFailure( |
374 | 327 | self._executor.run_priority_hook(invoke, "foobar"), | 327 | self._executor.run_priority_hook(invoke, "foobar"), |
375 | 328 | AssertionError) | 328 | AssertionError) |
377 | 329 | self.assertIn("Executor must not be running", str(error)) | 329 | self.assertEquals(str(error), "Executor must not be running") |
378 | 330 | 330 | ||
379 | 331 | @inlineCallbacks | 331 | @inlineCallbacks |
380 | 332 | def test_prioritize_with_queued(self): | 332 | def test_prioritize_with_queued(self): |
381 | 333 | 333 | ||
382 | === modified file 'ensemble/hooks/tests/test_invoker.py' | |||
383 | --- ensemble/hooks/tests/test_invoker.py 2011-08-16 17:26:09 +0000 | |||
384 | +++ ensemble/hooks/tests/test_invoker.py 2011-08-26 13:09:07 +0000 | |||
385 | @@ -480,7 +480,7 @@ | |||
386 | 480 | error = self.failUnlessRaises(errors.FormulaError, exe, hook) | 480 | error = self.failUnlessRaises(errors.FormulaError, exe, hook) |
387 | 481 | 481 | ||
388 | 482 | self.assertEqual(error.path, hook) | 482 | self.assertEqual(error.path, hook) |
390 | 483 | self.assertIn(error.message, "hook is not executable") | 483 | self.assertEqual(error.message, "hook is not executable") |
391 | 484 | 484 | ||
392 | 485 | @defer.inlineCallbacks | 485 | @defer.inlineCallbacks |
393 | 486 | def test_spawn_success(self): | 486 | def test_spawn_success(self): |
394 | 487 | 487 | ||
395 | === modified file 'ensemble/lib/tests/test_schema.py' | |||
396 | --- ensemble/lib/tests/test_schema.py 2011-07-27 08:54:46 +0000 | |||
397 | +++ ensemble/lib/tests/test_schema.py 2011-08-26 13:09:07 +0000 | |||
398 | @@ -183,7 +183,7 @@ | |||
399 | 183 | exp = "([a-" | 183 | exp = "([a-" |
400 | 184 | error = self.assertRaises( | 184 | error = self.assertRaises( |
401 | 185 | SchemaError, Regex().coerce, exp, PATH) | 185 | SchemaError, Regex().coerce, exp, PATH) |
403 | 186 | self.assertIn("expected regex", str(error)) | 186 | self.assertEquals(str(error), "<path>: expected regex, got '([a-'") |
404 | 187 | 187 | ||
405 | 188 | 188 | ||
406 | 189 | class ListTest(TestCase): | 189 | class ListTest(TestCase): |
407 | 190 | 190 | ||
408 | === modified file 'ensemble/machine/tests/test_unit_deployment.py' | |||
409 | --- ensemble/machine/tests/test_unit_deployment.py 2011-02-15 15:15:16 +0000 | |||
410 | +++ ensemble/machine/tests/test_unit_deployment.py 2011-08-26 13:09:07 +0000 | |||
411 | @@ -115,8 +115,8 @@ | |||
412 | 115 | 115 | ||
413 | 116 | self.failUnlessFailure(d, UnitDeploymentError) | 116 | self.failUnlessFailure(d, UnitDeploymentError) |
414 | 117 | 117 | ||
417 | 118 | def validate_result(result): | 118 | def validate_result(error): |
418 | 119 | self.assertIn("No module named magichat", str(result)) | 119 | self.assertIn("No module named magichat", str(error)) |
419 | 120 | 120 | ||
420 | 121 | d.addCallback(validate_result) | 121 | d.addCallback(validate_result) |
421 | 122 | return d | 122 | return d |
422 | @@ -170,7 +170,7 @@ | |||
423 | 170 | error = self.assertRaises( | 170 | error = self.assertRaises( |
424 | 171 | UnitDeploymentError, | 171 | UnitDeploymentError, |
425 | 172 | self.deployment.start, "0", get_test_zookeeper_address()) | 172 | self.deployment.start, "0", get_test_zookeeper_address()) |
427 | 173 | self.assertIn("Formula must be unpacked first.", str(error)) | 173 | self.assertEquals(str(error), "Formula must be unpacked first.") |
428 | 174 | 174 | ||
429 | 175 | @inlineCallbacks | 175 | @inlineCallbacks |
430 | 176 | def test_service_unit_destroy(self): | 176 | def test_service_unit_destroy(self): |
431 | @@ -269,7 +269,9 @@ | |||
432 | 269 | error = self.assertRaises( | 269 | error = self.assertRaises( |
433 | 270 | UnitDeploymentError, | 270 | UnitDeploymentError, |
434 | 271 | self.deployment.unpack_formula, self.formula) | 271 | self.deployment.unpack_formula, self.formula) |
436 | 272 | self.assertIn("Invalid formula for deployment:", str(error)) | 272 | self.assertEquals( |
437 | 273 | str(error), | ||
438 | 274 | "Invalid formula for deployment: %s" % self.formula.path) | ||
439 | 273 | 275 | ||
440 | 274 | def test_is_running_no_pid_file(self): | 276 | def test_is_running_no_pid_file(self): |
441 | 275 | """ | 277 | """ |
442 | 276 | 278 | ||
443 | === modified file 'ensemble/machine/unit.py' | |||
444 | --- ensemble/machine/unit.py 2011-02-15 15:15:16 +0000 | |||
445 | +++ ensemble/machine/unit.py 2011-08-26 13:09:07 +0000 | |||
446 | @@ -123,6 +123,6 @@ | |||
447 | 123 | """Unpack a formula to the service units directory.""" | 123 | """Unpack a formula to the service units directory.""" |
448 | 124 | if not isinstance(formula, FormulaBundle): | 124 | if not isinstance(formula, FormulaBundle): |
449 | 125 | raise UnitDeploymentError( | 125 | raise UnitDeploymentError( |
451 | 126 | "Invalid formula for deployment: %r" % formula) | 126 | "Invalid formula for deployment: %s" % formula.path) |
452 | 127 | 127 | ||
453 | 128 | formula.extract_to(os.path.join(self.directory, "formula")) | 128 | formula.extract_to(os.path.join(self.directory, "formula")) |
454 | 129 | 129 | ||
455 | === modified file 'ensemble/providers/common/connect.py' | |||
456 | --- ensemble/providers/common/connect.py 2011-08-23 08:42:30 +0000 | |||
457 | +++ ensemble/providers/common/connect.py 2011-08-26 13:09:07 +0000 | |||
458 | @@ -36,7 +36,7 @@ | |||
459 | 36 | for machine in machines: | 36 | for machine in machines: |
460 | 37 | if machine.dns_name: | 37 | if machine.dns_name: |
461 | 38 | return machine | 38 | return machine |
463 | 39 | raise EnvironmentPending("no machines have addresses assigned yet") | 39 | raise EnvironmentPending("No machines have addresses assigned yet") |
464 | 40 | 40 | ||
465 | 41 | def _connect_to_machine(self, machine, share): | 41 | def _connect_to_machine(self, machine, share): |
466 | 42 | log.info("Connecting to environment.") | 42 | log.info("Connecting to environment.") |
467 | @@ -48,7 +48,7 @@ | |||
468 | 48 | def _cannot_connect(self, failure, machine): | 48 | def _cannot_connect(self, failure, machine): |
469 | 49 | failure.trap(NoConnection, ConnectionTimeoutException) | 49 | failure.trap(NoConnection, ConnectionTimeoutException) |
470 | 50 | raise EnvironmentPending( | 50 | raise EnvironmentPending( |
472 | 51 | "machine %s may not be ready (%s)" | 51 | "Cannot connect to machine %s (perhaps still initializing): %s" |
473 | 52 | % (machine.instance_id, str(failure.value))) | 52 | % (machine.instance_id, str(failure.value))) |
474 | 53 | 53 | ||
475 | 54 | @inlineCallbacks | 54 | @inlineCallbacks |
476 | 55 | 55 | ||
477 | === modified file 'ensemble/providers/common/findzookeepers.py' | |||
478 | --- ensemble/providers/common/findzookeepers.py 2011-08-16 13:24:41 +0000 | |||
479 | +++ ensemble/providers/common/findzookeepers.py 2011-08-26 13:09:07 +0000 | |||
480 | @@ -26,5 +26,5 @@ | |||
481 | 26 | 26 | ||
482 | 27 | if machines: | 27 | if machines: |
483 | 28 | returnValue(machines) | 28 | returnValue(machines) |
485 | 29 | raise EnvironmentNotFound("machines are not running (%s)." | 29 | raise EnvironmentNotFound("machines are not running (%s)" |
486 | 30 | % ", ".join(missing_instance_ids)) | 30 | % ", ".join(missing_instance_ids)) |
487 | 31 | 31 | ||
488 | === modified file 'ensemble/providers/common/tests/test_connect.py' | |||
489 | --- ensemble/providers/common/tests/test_connect.py 2011-08-19 16:12:22 +0000 | |||
490 | +++ ensemble/providers/common/tests/test_connect.py 2011-08-26 13:09:07 +0000 | |||
491 | @@ -59,9 +59,7 @@ | |||
492 | 59 | 59 | ||
493 | 60 | def check(error): | 60 | def check(error): |
494 | 61 | self.assertEquals( | 61 | self.assertEquals( |
498 | 62 | str(error), | 62 | str(error), "No machines have addresses assigned yet") |
496 | 63 | "Ensemble environment is not accessible: no machines have " | ||
497 | 64 | "addresses assigned yet") | ||
499 | 65 | d.addCallback(check) | 63 | d.addCallback(check) |
500 | 66 | return d | 64 | return d |
501 | 67 | 65 | ||
502 | @@ -85,8 +83,8 @@ | |||
503 | 85 | """`NoConnection` errors should become `EnvironmentPending`s""" | 83 | """`NoConnection` errors should become `EnvironmentPending`s""" |
504 | 86 | return self.assert_connect_error( | 84 | return self.assert_connect_error( |
505 | 87 | NoConnection("KABOOM!"), EnvironmentPending, | 85 | NoConnection("KABOOM!"), EnvironmentPending, |
508 | 88 | "Ensemble environment is not accessible: machine i-exist may " | 86 | "Cannot connect to machine i-exist (perhaps still initializing): " |
509 | 89 | "not be ready (KABOOM!)") | 87 | "KABOOM!") |
510 | 90 | 88 | ||
511 | 91 | def test_txzookeeper_error(self): | 89 | def test_txzookeeper_error(self): |
512 | 92 | """ | 90 | """ |
513 | @@ -94,8 +92,8 @@ | |||
514 | 94 | """ | 92 | """ |
515 | 95 | return self.assert_connect_error( | 93 | return self.assert_connect_error( |
516 | 96 | ConnectionTimeoutException("SPLAT!"), EnvironmentPending, | 94 | ConnectionTimeoutException("SPLAT!"), EnvironmentPending, |
519 | 97 | "Ensemble environment is not accessible: machine i-exist may " | 95 | "Cannot connect to machine i-exist (perhaps still initializing): " |
520 | 98 | "not be ready (SPLAT!)") | 96 | "SPLAT!") |
521 | 99 | 97 | ||
522 | 100 | def test_other_error(self): | 98 | def test_other_error(self): |
523 | 101 | """Other errors should propagate""" | 99 | """Other errors should propagate""" |
524 | 102 | 100 | ||
525 | === modified file 'ensemble/providers/common/tests/test_findzookeepers.py' | |||
526 | --- ensemble/providers/common/tests/test_findzookeepers.py 2011-08-16 13:44:01 +0000 | |||
527 | +++ ensemble/providers/common/tests/test_findzookeepers.py 2011-08-26 13:09:07 +0000 | |||
528 | @@ -37,12 +37,27 @@ | |||
529 | 37 | provider = self.get_provider(False) | 37 | provider = self.get_provider(False) |
530 | 38 | d = provider.get_zookeeper_machines() | 38 | d = provider.get_zookeeper_machines() |
531 | 39 | self.assertFailure(d, EnvironmentNotFound) | 39 | self.assertFailure(d, EnvironmentNotFound) |
532 | 40 | |||
533 | 41 | def check(error): | ||
534 | 42 | self.assertEquals( | ||
535 | 43 | str(error), | ||
536 | 44 | "Ensemble environment not found: is the environment " | ||
537 | 45 | "bootstrapped?") | ||
538 | 46 | d.addCallback(check) | ||
539 | 40 | return d | 47 | return d |
540 | 41 | 48 | ||
541 | 42 | def test_empty_state(self): | 49 | def test_empty_state(self): |
542 | 43 | provider = self.get_provider({}) | 50 | provider = self.get_provider({}) |
543 | 44 | d = provider.get_zookeeper_machines() | 51 | d = provider.get_zookeeper_machines() |
544 | 45 | self.assertFailure(d, EnvironmentNotFound) | 52 | self.assertFailure(d, EnvironmentNotFound) |
545 | 53 | |||
546 | 54 | def check(error): | ||
547 | 55 | self.assertEquals( | ||
548 | 56 | str(error), | ||
549 | 57 | "Ensemble environment not found: is the environment " | ||
550 | 58 | "bootstrapped?") | ||
551 | 59 | d.addCallback(check) | ||
552 | 60 | return d | ||
553 | 46 | return d | 61 | return d |
554 | 47 | 62 | ||
555 | 48 | def test_get_machine_error_aborts(self): | 63 | def test_get_machine_error_aborts(self): |
556 | @@ -56,14 +71,24 @@ | |||
557 | 56 | self.assertFailure(d, SomeError) | 71 | self.assertFailure(d, SomeError) |
558 | 57 | return d | 72 | return d |
559 | 58 | 73 | ||
562 | 59 | def test_bad_machine(self): | 74 | def test_bad_machines(self): |
563 | 60 | provider = self.get_provider({"zookeeper-instances": ["porter"]}) | 75 | provider = self.get_provider({ |
564 | 76 | "zookeeper-instances": ["porter", "carter"]}) | ||
565 | 61 | provider.get_machines(["porter"]) | 77 | provider.get_machines(["porter"]) |
566 | 62 | self.mocker.result(fail(MachinesNotFound(["porter"]))) | 78 | self.mocker.result(fail(MachinesNotFound(["porter"]))) |
567 | 79 | provider.get_machines(["carter"]) | ||
568 | 80 | self.mocker.result(fail(MachinesNotFound(["carter"]))) | ||
569 | 63 | self.mocker.replay() | 81 | self.mocker.replay() |
570 | 64 | 82 | ||
571 | 65 | d = provider.get_zookeeper_machines() | 83 | d = provider.get_zookeeper_machines() |
572 | 66 | self.assertFailure(d, EnvironmentNotFound) | 84 | self.assertFailure(d, EnvironmentNotFound) |
573 | 85 | |||
574 | 86 | def check(error): | ||
575 | 87 | self.assertEquals( | ||
576 | 88 | str(error), | ||
577 | 89 | "Ensemble environment not found: machines are not running " | ||
578 | 90 | "(porter, carter)") | ||
579 | 91 | d.addCallback(check) | ||
580 | 67 | return d | 92 | return d |
581 | 68 | 93 | ||
582 | 69 | def test_good_machine(self): | 94 | def test_good_machine(self): |
583 | 70 | 95 | ||
584 | === modified file 'ensemble/providers/common/tests/test_utils.py' | |||
585 | --- ensemble/providers/common/tests/test_utils.py 2011-08-11 22:04:38 +0000 | |||
586 | +++ ensemble/providers/common/tests/test_utils.py 2011-08-26 13:09:07 +0000 | |||
587 | @@ -63,9 +63,7 @@ | |||
588 | 63 | OSError("Bad")) | 63 | OSError("Bad")) |
589 | 64 | self.assertInstance(error, EnsembleError) | 64 | self.assertInstance(error, EnsembleError) |
590 | 65 | self.assertEqual( | 65 | self.assertEqual( |
594 | 66 | str(error), | 66 | str(error), "Unexpected OSError interacting with provider: Bad") |
592 | 67 | "ProviderError: Interaction with machine provider failed: " | ||
593 | 68 | "OSError('Bad',)") | ||
595 | 69 | 67 | ||
596 | 70 | def test_convert_unknown_error_ignores_ensemble_error(self): | 68 | def test_convert_unknown_error_ignores_ensemble_error(self): |
597 | 71 | error = self.assertRaises( | 69 | error = self.assertRaises( |
598 | @@ -87,8 +85,7 @@ | |||
599 | 87 | self.assertInstance(failure.value, ProviderInteractionError) | 85 | self.assertInstance(failure.value, ProviderInteractionError) |
600 | 88 | self.assertEqual( | 86 | self.assertEqual( |
601 | 89 | str(failure.value), | 87 | str(failure.value), |
604 | 90 | "ProviderError: Interaction with machine provider failed: " | 88 | "Unexpected OSError interacting with provider: Bad") |
603 | 91 | "OSError('Bad',)") | ||
605 | 92 | 89 | ||
606 | 93 | 90 | ||
607 | 94 | class FormatCloudInitTest(TestCase): | 91 | class FormatCloudInitTest(TestCase): |
608 | 95 | 92 | ||
609 | === modified file 'ensemble/providers/common/utils.py' | |||
610 | --- ensemble/providers/common/utils.py 2011-08-03 15:53:38 +0000 | |||
611 | +++ ensemble/providers/common/utils.py 2011-08-26 13:09:07 +0000 | |||
612 | @@ -31,7 +31,9 @@ | |||
613 | 31 | error = failure | 31 | error = failure |
614 | 32 | 32 | ||
615 | 33 | if not isinstance(error, EnsembleError): | 33 | if not isinstance(error, EnsembleError): |
617 | 34 | error = ProviderInteractionError(error) | 34 | message = ("Unexpected %s interacting with provider: %s" |
618 | 35 | % (type(error).__name__, str(error))) | ||
619 | 36 | error = ProviderInteractionError(message) | ||
620 | 35 | 37 | ||
621 | 36 | if isinstance(failure, Failure): | 38 | if isinstance(failure, Failure): |
622 | 37 | return Failure(error) | 39 | return Failure(error) |
623 | 38 | 40 | ||
624 | === modified file 'ensemble/providers/ec2/__init__.py' | |||
625 | --- ensemble/providers/ec2/__init__.py 2011-08-25 16:41:03 +0000 | |||
626 | +++ ensemble/providers/ec2/__init__.py 2011-08-26 13:09:07 +0000 | |||
627 | @@ -77,13 +77,14 @@ | |||
628 | 77 | instances = yield self.ec2.describe_instances(*instance_ids) | 77 | instances = yield self.ec2.describe_instances(*instance_ids) |
629 | 78 | except EC2Error as error: | 78 | except EC2Error as error: |
630 | 79 | code = error.get_error_codes() | 79 | code = error.get_error_codes() |
631 | 80 | message = error.get_error_messages() | ||
632 | 80 | if code == "InvalidInstanceID.NotFound": | 81 | if code == "InvalidInstanceID.NotFound": |
633 | 81 | message = error.get_error_messages() | 82 | message = error.get_error_messages() |
634 | 82 | raise MachinesNotFound( | 83 | raise MachinesNotFound( |
635 | 83 | re.findall(r"\bi-[0-9a-f]{3,15}\b", message)) | 84 | re.findall(r"\bi-[0-9a-f]{3,15}\b", message)) |
636 | 84 | raise ProviderInteractionError( | 85 | raise ProviderInteractionError( |
639 | 85 | "EC2 error when looking up instances %s: %s" | 86 | "Unexpected EC2Error getting machines %s: %s" |
640 | 86 | % (", ".join(instance_ids), error.get_error_messages())) | 87 | % (", ".join(instance_ids), message)) |
641 | 87 | 88 | ||
642 | 88 | machines = [] | 89 | machines = [] |
643 | 89 | for instance in instances: | 90 | for instance in instances: |
644 | 90 | 91 | ||
645 | === modified file 'ensemble/providers/ec2/launch.py' | |||
646 | --- ensemble/providers/ec2/launch.py 2011-08-10 17:23:06 +0000 | |||
647 | +++ ensemble/providers/ec2/launch.py 2011-08-26 13:09:07 +0000 | |||
648 | @@ -123,9 +123,8 @@ | |||
649 | 123 | log.debug("Cannot delete security group %s: %s", | 123 | log.debug("Cannot delete security group %s: %s", |
650 | 124 | ensemble_machine_group, e) | 124 | ensemble_machine_group, e) |
651 | 125 | raise ProviderInteractionError( | 125 | raise ProviderInteractionError( |
655 | 126 | "EC2 error when attempting to delete " | 126 | "Unexpected EC2Error deleting security group %s: %s" |
656 | 127 | "security group %s: %s" % ( | 127 | % (ensemble_machine_group, e.get_error_messages())) |
654 | 128 | ensemble_machine_group, e)) | ||
657 | 129 | log.debug("Creating ensemble machine security group %s", | 128 | log.debug("Creating ensemble machine security group %s", |
658 | 130 | ensemble_machine_group) | 129 | ensemble_machine_group) |
659 | 131 | yield self._provider.ec2.create_security_group( | 130 | yield self._provider.ec2.create_security_group( |
660 | 132 | 131 | ||
661 | === modified file 'ensemble/providers/ec2/securitygroup.py' | |||
662 | --- ensemble/providers/ec2/securitygroup.py 2011-08-10 17:23:06 +0000 | |||
663 | +++ ensemble/providers/ec2/securitygroup.py 2011-08-26 13:09:07 +0000 | |||
664 | @@ -38,8 +38,8 @@ | |||
665 | 38 | port, protocol, machine.instance_id) | 38 | port, protocol, machine.instance_id) |
666 | 39 | except EC2Error, e: | 39 | except EC2Error, e: |
667 | 40 | raise ProviderInteractionError( | 40 | raise ProviderInteractionError( |
670 | 41 | "EC2 error when attempting to open %s/%s on machine %s: %s" % ( | 41 | "Unexpected EC2Error opening %s/%s on machine %s: %s" |
671 | 42 | port, protocol, machine.instance_id, e)) | 42 | % (port, protocol, machine.instance_id, e.get_error_messages())) |
672 | 43 | 43 | ||
673 | 44 | 44 | ||
674 | 45 | @inlineCallbacks | 45 | @inlineCallbacks |
675 | @@ -55,8 +55,8 @@ | |||
676 | 55 | port, protocol, machine.instance_id) | 55 | port, protocol, machine.instance_id) |
677 | 56 | except EC2Error, e: | 56 | except EC2Error, e: |
678 | 57 | raise ProviderInteractionError( | 57 | raise ProviderInteractionError( |
681 | 58 | "EC2 error when attempting to close %s/%s on machine %s: %s" % ( | 58 | "Unexpected EC2Error closing %s/%s on machine %s: %s" |
682 | 59 | port, protocol, machine.instance_id, e)) | 59 | % (port, protocol, machine.instance_id, e.get_error_messages())) |
683 | 60 | 60 | ||
684 | 61 | 61 | ||
685 | 62 | @inlineCallbacks | 62 | @inlineCallbacks |
686 | @@ -72,9 +72,8 @@ | |||
687 | 72 | _get_machine_group_name(provider, machine_id)) | 72 | _get_machine_group_name(provider, machine_id)) |
688 | 73 | except EC2Error, e: | 73 | except EC2Error, e: |
689 | 74 | raise ProviderInteractionError( | 74 | raise ProviderInteractionError( |
693 | 75 | "EC2 error when attempting to get opened ports " | 75 | "Unexpected EC2Error getting open ports on machine %s: %s" |
694 | 76 | "on machine %s: %s" % ( | 76 | % (machine.instance_id, e.get_error_messages())) |
692 | 77 | machine.instance_id, e)) | ||
695 | 78 | 77 | ||
696 | 79 | opened_ports = set() # made up of (port, protocol) pairs | 78 | opened_ports = set() # made up of (port, protocol) pairs |
697 | 80 | for ip_permission in security_groups[0].allowed_ips: | 79 | for ip_permission in security_groups[0].allowed_ips: |
698 | 81 | 80 | ||
699 | === modified file 'ensemble/providers/ec2/tests/common.py' | |||
700 | --- ensemble/providers/ec2/tests/common.py 2011-08-22 23:03:03 +0000 | |||
701 | +++ ensemble/providers/ec2/tests/common.py 2011-08-26 13:09:07 +0000 | |||
702 | @@ -59,16 +59,6 @@ | |||
703 | 59 | "<error><Code>1</Code><Message>%s</Message></error>" % message, | 59 | "<error><Code>1</Code><Message>%s</Message></error>" % message, |
704 | 60 | code) | 60 | code) |
705 | 61 | 61 | ||
706 | 62 | def get_wrapped_ec2_error_text(self, entity_id, reason, | ||
707 | 63 | format="The instance ID %r does not exist"): | ||
708 | 64 | """By convention, `EC2Error` is wrapped as a `ProviderError`""" | ||
709 | 65 | message = format % entity_id | ||
710 | 66 | return ( | ||
711 | 67 | "ProviderError: Interaction with machine provider failed: " | ||
712 | 68 | "\"EC2 error when attempting to %s %s: " | ||
713 | 69 | "Error Message: %s\"" % ( | ||
714 | 70 | reason, entity_id, message)) | ||
715 | 71 | |||
716 | 72 | def setUp(self): | 62 | def setUp(self): |
717 | 73 | # mock out the aws services | 63 | # mock out the aws services |
718 | 74 | service_factory = self.mocker.replace( | 64 | service_factory = self.mocker.replace( |
719 | 75 | 65 | ||
720 | === modified file 'ensemble/providers/ec2/tests/test_connect.py' | |||
721 | --- ensemble/providers/ec2/tests/test_connect.py 2011-08-19 16:12:22 +0000 | |||
722 | +++ ensemble/providers/ec2/tests/test_connect.py 2011-08-26 13:09:07 +0000 | |||
723 | @@ -59,9 +59,7 @@ | |||
724 | 59 | 59 | ||
725 | 60 | def check_error(error): | 60 | def check_error(error): |
726 | 61 | self.assertEqual( | 61 | self.assertEqual( |
730 | 62 | str(error), | 62 | str(error), "No machines have addresses assigned yet") |
728 | 63 | "Ensemble environment is not accessible: no machines have " | ||
729 | 64 | "addresses assigned yet") | ||
731 | 65 | 63 | ||
732 | 66 | self.assertFailure(d, NoConnection) | 64 | self.assertFailure(d, NoConnection) |
733 | 67 | d.addCallback(check_error) | 65 | d.addCallback(check_error) |
734 | @@ -89,8 +87,8 @@ | |||
735 | 89 | """`NoConnection` errors should become `EnvironmentPending`s""" | 87 | """`NoConnection` errors should become `EnvironmentPending`s""" |
736 | 90 | return self.assert_connect_error( | 88 | return self.assert_connect_error( |
737 | 91 | NoConnection("KABOOM!"), EnvironmentPending, | 89 | NoConnection("KABOOM!"), EnvironmentPending, |
740 | 92 | "Ensemble environment is not accessible: machine i-foobar may " | 90 | "Cannot connect to machine i-foobar (perhaps still initializing): " |
741 | 93 | "not be ready (KABOOM!)") | 91 | "KABOOM!") |
742 | 94 | 92 | ||
743 | 95 | def test_txzookeeper_error(self): | 93 | def test_txzookeeper_error(self): |
744 | 96 | """ | 94 | """ |
745 | @@ -98,8 +96,8 @@ | |||
746 | 98 | """ | 96 | """ |
747 | 99 | return self.assert_connect_error( | 97 | return self.assert_connect_error( |
748 | 100 | ConnectionTimeoutException("SPLAT!"), EnvironmentPending, | 98 | ConnectionTimeoutException("SPLAT!"), EnvironmentPending, |
751 | 101 | "Ensemble environment is not accessible: machine i-foobar may " | 99 | "Cannot connect to machine i-foobar (perhaps still initializing): " |
752 | 102 | "not be ready (SPLAT!)") | 100 | "SPLAT!") |
753 | 103 | 101 | ||
754 | 104 | def test_other_error(self): | 102 | def test_other_error(self): |
755 | 105 | """Other errors should propagate""" | 103 | """Other errors should propagate""" |
756 | 106 | 104 | ||
757 | === modified file 'ensemble/providers/ec2/tests/test_getmachines.py' | |||
758 | --- ensemble/providers/ec2/tests/test_getmachines.py 2011-08-17 08:58:51 +0000 | |||
759 | +++ ensemble/providers/ec2/tests/test_getmachines.py 2011-08-26 13:09:07 +0000 | |||
760 | @@ -132,9 +132,8 @@ | |||
761 | 132 | def verify(error): | 132 | def verify(error): |
762 | 133 | self.assertEquals( | 133 | self.assertEquals( |
763 | 134 | str(error), | 134 | str(error), |
767 | 135 | "ProviderError: Interaction with machine provider failed: " | 135 | "Unexpected EC2Error getting machines i-brokeit, i-msorry: " |
768 | 136 | "\"EC2 error when looking up instances i-brokeit, i-msorry: " | 136 | "unhelpful noises ('splat! kerpow!')") |
766 | 137 | "unhelpful noises ('splat! kerpow!')\"") | ||
769 | 138 | d.addCallback(verify) | 137 | d.addCallback(verify) |
770 | 139 | return d | 138 | return d |
771 | 140 | 139 | ||
772 | 141 | 140 | ||
773 | === modified file 'ensemble/providers/ec2/tests/test_launch.py' | |||
774 | --- ensemble/providers/ec2/tests/test_launch.py 2011-08-16 13:55:11 +0000 | |||
775 | +++ ensemble/providers/ec2/tests/test_launch.py 2011-08-26 13:09:07 +0000 | |||
776 | @@ -141,10 +141,9 @@ | |||
777 | 141 | ProviderInteractionError) | 141 | ProviderInteractionError) |
778 | 142 | self.assertEqual( | 142 | self.assertEqual( |
779 | 143 | str(ex), | 143 | str(ex), |
784 | 144 | self.get_wrapped_ec2_error_text( | 144 | "Unexpected EC2Error deleting security group " |
785 | 145 | "ensemble-moon-machine-1", "delete security group", | 145 | "ensemble-moon-machine-1: There are active instances using " |
786 | 146 | "There are active instances using security group %r" | 146 | "security group 'ensemble-moon-machine-1'") |
783 | 147 | )) | ||
787 | 148 | 147 | ||
788 | 149 | def test_provider_type_machine_variable(self): | 148 | def test_provider_type_machine_variable(self): |
789 | 150 | """The provider type is available via cloud-init.""" | 149 | """The provider type is available via cloud-init.""" |
790 | 151 | 150 | ||
791 | === modified file 'ensemble/providers/ec2/tests/test_provider.py' | |||
792 | --- ensemble/providers/ec2/tests/test_provider.py 2011-08-11 19:49:32 +0000 | |||
793 | +++ ensemble/providers/ec2/tests/test_provider.py 2011-08-26 13:09:07 +0000 | |||
794 | @@ -127,4 +127,7 @@ | |||
795 | 127 | config["authorized-keys-path"] = "File path" | 127 | config["authorized-keys-path"] = "File path" |
796 | 128 | error = self.assertRaises(EnvironmentsConfigError, | 128 | error = self.assertRaises(EnvironmentsConfigError, |
797 | 129 | MachineProvider, "some-env-name", config) | 129 | MachineProvider, "some-env-name", config) |
799 | 130 | self.assertIn("authorized-keys", str(error)) | 130 | self.assertEquals( |
800 | 131 | str(error), | ||
801 | 132 | "Environment config cannot define both authorized-keys and " | ||
802 | 133 | "authorized-keys-path. Pick one!") | ||
803 | 131 | 134 | ||
804 | === modified file 'ensemble/providers/ec2/tests/test_securitygroup.py' | |||
805 | --- ensemble/providers/ec2/tests/test_securitygroup.py 2011-08-10 17:19:09 +0000 | |||
806 | +++ ensemble/providers/ec2/tests/test_securitygroup.py 2011-08-26 13:09:07 +0000 | |||
807 | @@ -89,8 +89,8 @@ | |||
808 | 89 | ProviderInteractionError) | 89 | ProviderInteractionError) |
809 | 90 | self.assertEqual( | 90 | self.assertEqual( |
810 | 91 | str(ex), | 91 | str(ex), |
813 | 92 | self.get_wrapped_ec2_error_text( | 92 | "Unexpected EC2Error opening 80/tcp on machine i-foobar: " |
814 | 93 | "i-foobar", "open 80/tcp on machine")) | 93 | "The instance ID 'i-foobar' does not exist") |
815 | 94 | 94 | ||
816 | 95 | @inlineCallbacks | 95 | @inlineCallbacks |
817 | 96 | def test_close_provider_port_unknown_instance(self): | 96 | def test_close_provider_port_unknown_instance(self): |
818 | @@ -108,8 +108,8 @@ | |||
819 | 108 | ProviderInteractionError) | 108 | ProviderInteractionError) |
820 | 109 | self.assertEqual( | 109 | self.assertEqual( |
821 | 110 | str(ex), | 110 | str(ex), |
824 | 111 | self.get_wrapped_ec2_error_text( | 111 | "Unexpected EC2Error closing 80/tcp on machine i-foobar: " |
825 | 112 | "i-foobar", "close 80/tcp on machine")) | 112 | "The instance ID 'i-foobar' does not exist") |
826 | 113 | 113 | ||
827 | 114 | @inlineCallbacks | 114 | @inlineCallbacks |
828 | 115 | def test_get_provider_opened_ports_unknown_instance(self): | 115 | def test_get_provider_opened_ports_unknown_instance(self): |
829 | @@ -125,5 +125,5 @@ | |||
830 | 125 | ProviderInteractionError) | 125 | ProviderInteractionError) |
831 | 126 | self.assertEqual( | 126 | self.assertEqual( |
832 | 127 | str(ex), | 127 | str(ex), |
835 | 128 | self.get_wrapped_ec2_error_text( | 128 | "Unexpected EC2Error getting open ports on machine i-foobar: " |
836 | 129 | "i-foobar", "get opened ports on machine")) | 129 | "The instance ID 'i-foobar' does not exist") |
837 | 130 | 130 | ||
838 | === modified file 'ensemble/providers/orchestra/cobbler.py' | |||
839 | --- ensemble/providers/orchestra/cobbler.py 2011-08-22 13:01:02 +0000 | |||
840 | +++ ensemble/providers/orchestra/cobbler.py 2011-08-26 13:09:07 +0000 | |||
841 | @@ -3,8 +3,7 @@ | |||
842 | 3 | from twisted.internet.defer import inlineCallbacks, returnValue | 3 | from twisted.internet.defer import inlineCallbacks, returnValue |
843 | 4 | from twisted.web.xmlrpc import Proxy | 4 | from twisted.web.xmlrpc import Proxy |
844 | 5 | 5 | ||
847 | 6 | from ensemble.errors import ( | 6 | from ensemble.errors import MachinesNotFound, ProviderError |
846 | 7 | MachinesNotFound, ProviderError, ProviderInteractionError) | ||
848 | 8 | 7 | ||
849 | 9 | 8 | ||
850 | 10 | class CobblerCaller(object): | 9 | class CobblerCaller(object): |
851 | @@ -62,7 +61,7 @@ | |||
852 | 62 | def check(actual): | 61 | def check(actual): |
853 | 63 | if actual != expect: | 62 | if actual != expect: |
854 | 64 | raise ProviderError( | 63 | raise ProviderError( |
856 | 65 | "Bad result from call to %s with %s (got %r, expected %r)" | 64 | "Bad result from call to %s with %s: got %r, expected %r" |
857 | 66 | % (name, args, actual, expect)) | 65 | % (name, args, actual, expect)) |
858 | 67 | return actual | 66 | return actual |
859 | 68 | 67 | ||
860 | @@ -84,8 +83,9 @@ | |||
861 | 84 | 83 | ||
862 | 85 | def extract_name(systems): | 84 | def extract_name(systems): |
863 | 86 | if len(systems) > 1: | 85 | if len(systems) > 1: |
866 | 87 | raise ProviderInteractionError( | 86 | raise ProviderError( |
867 | 88 | "Got multiple instances with id %s" % instance_id) | 87 | "Got multiple names for machine %s: %s" |
868 | 88 | % (instance_id, ", ".join(systems))) | ||
869 | 89 | if not systems: | 89 | if not systems: |
870 | 90 | raise MachinesNotFound([instance_id]) | 90 | raise MachinesNotFound([instance_id]) |
871 | 91 | return systems[0] | 91 | return systems[0] |
872 | 92 | 92 | ||
873 | === modified file 'ensemble/providers/orchestra/tests/test_cobbler.py' | |||
874 | --- ensemble/providers/orchestra/tests/test_cobbler.py 2011-08-12 21:35:34 +0000 | |||
875 | +++ ensemble/providers/orchestra/tests/test_cobbler.py 2011-08-26 13:09:07 +0000 | |||
876 | @@ -3,8 +3,7 @@ | |||
877 | 3 | from twisted.internet.defer import fail, succeed | 3 | from twisted.internet.defer import fail, succeed |
878 | 4 | from twisted.web.xmlrpc import Proxy | 4 | from twisted.web.xmlrpc import Proxy |
879 | 5 | 5 | ||
882 | 6 | from ensemble.errors import ( | 6 | from ensemble.errors import MachinesNotFound, ProviderError |
881 | 7 | MachinesNotFound, ProviderError, ProviderInteractionError) | ||
883 | 8 | from ensemble.lib.testing import TestCase | 7 | from ensemble.lib.testing import TestCase |
884 | 9 | from ensemble.providers.orchestra.cobbler import CobblerCaller, CobblerClient | 8 | from ensemble.providers.orchestra.cobbler import CobblerCaller, CobblerClient |
885 | 10 | 9 | ||
886 | @@ -243,11 +242,14 @@ | |||
887 | 243 | d.addCallback(verify) | 242 | d.addCallback(verify) |
888 | 244 | return d | 243 | return d |
889 | 245 | 244 | ||
891 | 246 | def check_bad_call(self, d, method): | 245 | def check_bad_call(self, d, method, args): |
892 | 247 | self.assertFailure(d, ProviderError) | 246 | self.assertFailure(d, ProviderError) |
893 | 248 | 247 | ||
894 | 249 | def verify(error): | 248 | def verify(error): |
896 | 250 | self.assertIn("Bad result from call to %s" % method, str(error)) | 249 | self.assertEquals( |
897 | 250 | str(error), | ||
898 | 251 | "Bad result from call to %s with %s: got False, expected True" | ||
899 | 252 | % (method, args)) | ||
900 | 251 | d.addCallback(verify) | 253 | d.addCallback(verify) |
901 | 252 | return d | 254 | return d |
902 | 253 | 255 | ||
903 | @@ -261,11 +263,13 @@ | |||
904 | 261 | self.mock_get_name(succeed(["some-name", "another-name"])) | 263 | self.mock_get_name(succeed(["some-name", "another-name"])) |
905 | 262 | self.mocker.replay() | 264 | self.mocker.replay() |
906 | 263 | d = self.call_cobbler_method(method_name, *args) | 265 | d = self.call_cobbler_method(method_name, *args) |
908 | 264 | self.assertFailure(d, ProviderInteractionError) | 266 | self.assertFailure(d, ProviderError) |
909 | 265 | 267 | ||
910 | 266 | def check_error(error): | 268 | def check_error(error): |
913 | 267 | self.assertIn("Got multiple instances with id some-uid", | 269 | self.assertEquals( |
914 | 268 | str(error)) | 270 | str(error), |
915 | 271 | "Got multiple names for machine some-uid: some-name, " | ||
916 | 272 | "another-name") | ||
917 | 269 | d.addCallback(check_error) | 273 | d.addCallback(check_error) |
918 | 270 | return d | 274 | return d |
919 | 271 | 275 | ||
920 | @@ -301,7 +305,8 @@ | |||
921 | 301 | self.mock_modify_system(succeed(False), key, value) | 305 | self.mock_modify_system(succeed(False), key, value) |
922 | 302 | self.mocker.replay() | 306 | self.mocker.replay() |
923 | 303 | d = self.call_cobbler_method(method_name, *args) | 307 | d = self.call_cobbler_method(method_name, *args) |
925 | 304 | return self.check_bad_call(d, "modify_system") | 308 | return self.check_bad_call( |
926 | 309 | d, "modify_system", ("some-handle", key, value)) | ||
927 | 305 | 310 | ||
928 | 306 | def check_modify_system_error(self, key, value, method_name, *args): | 311 | def check_modify_system_error(self, key, value, method_name, *args): |
929 | 307 | self.mock_modify_system(fail(SomeError()), key, value) | 312 | self.mock_modify_system(fail(SomeError()), key, value) |
930 | @@ -314,7 +319,7 @@ | |||
931 | 314 | self.mock_save_system(succeed(False)) | 319 | self.mock_save_system(succeed(False)) |
932 | 315 | self.mocker.replay() | 320 | self.mocker.replay() |
933 | 316 | d = self.call_cobbler_method(method_name, *args) | 321 | d = self.call_cobbler_method(method_name, *args) |
935 | 317 | return self.check_bad_call(d, "save_system") | 322 | return self.check_bad_call(d, "save_system", ("some-handle",)) |
936 | 318 | 323 | ||
937 | 319 | def check_save_system_error(self, method_name, *args): | 324 | def check_save_system_error(self, method_name, *args): |
938 | 320 | self.mock_save_system(fail(SomeError())) | 325 | self.mock_save_system(fail(SomeError())) |
939 | 321 | 326 | ||
940 | === modified file 'ensemble/providers/orchestra/tests/test_connect.py' | |||
941 | --- ensemble/providers/orchestra/tests/test_connect.py 2011-08-19 16:12:22 +0000 | |||
942 | +++ ensemble/providers/orchestra/tests/test_connect.py 2011-08-26 13:09:07 +0000 | |||
943 | @@ -62,8 +62,8 @@ | |||
944 | 62 | """`NoConnection` errors should become `EnvironmentPending`s""" | 62 | """`NoConnection` errors should become `EnvironmentPending`s""" |
945 | 63 | return self.assert_connect_error( | 63 | return self.assert_connect_error( |
946 | 64 | NoConnection("KABOOM!"), EnvironmentPending, | 64 | NoConnection("KABOOM!"), EnvironmentPending, |
949 | 65 | "Ensemble environment is not accessible: machine foo may not be " | 65 | "Cannot connect to machine foo (perhaps still initializing): " |
950 | 66 | "ready (KABOOM!)") | 66 | "KABOOM!") |
951 | 67 | 67 | ||
952 | 68 | def test_txzookeeper_error(self): | 68 | def test_txzookeeper_error(self): |
953 | 69 | """ | 69 | """ |
954 | @@ -71,8 +71,8 @@ | |||
955 | 71 | """ | 71 | """ |
956 | 72 | return self.assert_connect_error( | 72 | return self.assert_connect_error( |
957 | 73 | ConnectionTimeoutException("SPLAT!"), EnvironmentPending, | 73 | ConnectionTimeoutException("SPLAT!"), EnvironmentPending, |
960 | 74 | "Ensemble environment is not accessible: machine foo may not be " | 74 | "Cannot connect to machine foo (perhaps still initializing): " |
961 | 75 | "ready (SPLAT!)") | 75 | "SPLAT!") |
962 | 76 | 76 | ||
963 | 77 | def test_other_error(self): | 77 | def test_other_error(self): |
964 | 78 | """Other errors should propagate""" | 78 | """Other errors should propagate""" |
965 | 79 | 79 | ||
966 | === modified file 'ensemble/providers/orchestra/tests/test_launch.py' | |||
967 | --- ensemble/providers/orchestra/tests/test_launch.py 2011-08-11 23:27:45 +0000 | |||
968 | +++ ensemble/providers/orchestra/tests/test_launch.py 2011-08-26 13:09:07 +0000 | |||
969 | @@ -30,9 +30,13 @@ | |||
970 | 30 | 30 | ||
971 | 31 | d = self.get_provider().start_machine({"machine-id": "32767"}) | 31 | d = self.get_provider().start_machine({"machine-id": "32767"}) |
972 | 32 | self.assertFailure(d, ProviderError) | 32 | self.assertFailure(d, ProviderError) |
976 | 33 | self.assertIn("Could not find any Cobbler systems marked as available " | 33 | |
977 | 34 | "and configured for network boot.", | 34 | def verify(error): |
978 | 35 | str(d)) | 35 | self.assertEquals( |
979 | 36 | str(error), | ||
980 | 37 | "Could not find any Cobbler systems marked as available and " | ||
981 | 38 | "configured for network boot.") | ||
982 | 39 | d.addCallback(verify) | ||
983 | 36 | return d | 40 | return d |
984 | 37 | 41 | ||
985 | 38 | def test_no_acceptable_machines(self): | 42 | def test_no_acceptable_machines(self): |
986 | @@ -43,9 +47,13 @@ | |||
987 | 43 | 47 | ||
988 | 44 | d = self.get_provider().start_machine({"machine-id": "32767"}) | 48 | d = self.get_provider().start_machine({"machine-id": "32767"}) |
989 | 45 | self.assertFailure(d, ProviderError) | 49 | self.assertFailure(d, ProviderError) |
993 | 46 | self.assertIn("All available Cobbler systems were also marked as " | 50 | |
994 | 47 | "acquired (instances: bad-system-uid).", | 51 | def verify(error): |
995 | 48 | str(d)) | 52 | self.assertEquals( |
996 | 53 | str(error), | ||
997 | 54 | "All available Cobbler systems were also marked as acquired " | ||
998 | 55 | "(instances: bad-system-uid).") | ||
999 | 56 | d.addCallback(verify) | ||
1000 | 49 | return d | 57 | return d |
1001 | 50 | 58 | ||
1002 | 51 | def test_actually_launch(self): | 59 | def test_actually_launch(self): |
1003 | 52 | 60 | ||
1004 | === modified file 'ensemble/providers/orchestra/tests/test_provider.py' | |||
1005 | --- ensemble/providers/orchestra/tests/test_provider.py 2011-08-23 16:07:09 +0000 | |||
1006 | +++ ensemble/providers/orchestra/tests/test_provider.py 2011-08-26 13:09:07 +0000 | |||
1007 | @@ -53,7 +53,10 @@ | |||
1008 | 53 | config["authorized-keys-path"] = "File path" | 53 | config["authorized-keys-path"] = "File path" |
1009 | 54 | error = self.assertRaises(EnvironmentsConfigError, | 54 | error = self.assertRaises(EnvironmentsConfigError, |
1010 | 55 | MachineProvider, "some-env-name", config) | 55 | MachineProvider, "some-env-name", config) |
1012 | 56 | self.assertIn("authorized-keys", str(error)) | 56 | self.assertEquals( |
1013 | 57 | str(error), | ||
1014 | 58 | "Environment config cannot define both authorized-keys and " | ||
1015 | 59 | "authorized-keys-path. Pick one!") | ||
1016 | 57 | 60 | ||
1017 | 58 | @inlineCallbacks | 61 | @inlineCallbacks |
1018 | 59 | def test_open_port(self): | 62 | def test_open_port(self): |
1019 | @@ -75,4 +78,3 @@ | |||
1020 | 75 | yield MachineProvider("blah", CONFIG).get_opened_ports(None, None) | 78 | yield MachineProvider("blah", CONFIG).get_opened_ports(None, None) |
1021 | 76 | self.assertIn( | 79 | self.assertIn( |
1022 | 77 | "Firewalling is not yet implemented in Orchestra", log.getvalue()) | 80 | "Firewalling is not yet implemented in Orchestra", log.getvalue()) |
1023 | 78 | |||
1024 | 79 | 81 | ||
1025 | === modified file 'ensemble/state/service.py' | |||
1026 | --- ensemble/state/service.py 2011-08-08 03:22:15 +0000 | |||
1027 | +++ ensemble/state/service.py 2011-08-26 13:09:07 +0000 | |||
1028 | @@ -855,7 +855,8 @@ | |||
1029 | 855 | cleared. | 855 | cleared. |
1030 | 856 | """ | 856 | """ |
1031 | 857 | if not isinstance(hook_names, (list, tuple)): | 857 | if not isinstance(hook_names, (list, tuple)): |
1033 | 858 | raise AssertionError("hook names must be a list %r" % hook_names) | 858 | raise AssertionError("Hook names must be a list: got %r" |
1034 | 859 | % hook_names) | ||
1035 | 859 | 860 | ||
1036 | 860 | if "*" in hook_names and len(hook_names) > 1: | 861 | if "*" in hook_names and len(hook_names) > 1: |
1037 | 861 | msg = "Ambigious to debug all hooks and named hooks %r" % ( | 862 | msg = "Ambigious to debug all hooks and named hooks %r" % ( |
1038 | 862 | 863 | ||
1039 | === modified file 'ensemble/state/tests/test_security.py' | |||
1040 | --- ensemble/state/tests/test_security.py 2011-08-05 22:55:52 +0000 | |||
1041 | +++ ensemble/state/tests/test_security.py 2011-08-26 13:09:07 +0000 | |||
1042 | @@ -316,7 +316,7 @@ | |||
1043 | 316 | principal = Principal("zebra", "zoo") | 316 | principal = Principal("zebra", "zoo") |
1044 | 317 | error = yield self.assertFailure(self.db.get(principal.name), | 317 | error = yield self.assertFailure(self.db.get(principal.name), |
1045 | 318 | PrincipalNotFound) | 318 | PrincipalNotFound) |
1047 | 319 | self.assertIn("zebra", str(error)) | 319 | self.assertEquals(str(error), "Principal 'zebra' not found") |
1048 | 320 | 320 | ||
1049 | 321 | 321 | ||
1050 | 322 | class PolicyTest(TestCase): | 322 | class PolicyTest(TestCase): |
1051 | 323 | 323 | ||
1052 | === modified file 'ensemble/state/tests/test_service.py' | |||
1053 | --- ensemble/state/tests/test_service.py 2011-08-22 23:02:38 +0000 | |||
1054 | +++ ensemble/state/tests/test_service.py 2011-08-26 13:09:07 +0000 | |||
1055 | @@ -1111,8 +1111,10 @@ | |||
1056 | 1111 | error = yield self.assertFailure( | 1111 | error = yield self.assertFailure( |
1057 | 1112 | unit_state.enable_hook_debug(["*", "db-relation-changed"]), | 1112 | unit_state.enable_hook_debug(["*", "db-relation-changed"]), |
1058 | 1113 | ValueError) | 1113 | ValueError) |
1061 | 1114 | self.assertIn("Ambigious to debug all hooks and named hooks", | 1114 | self.assertEquals( |
1062 | 1115 | str(error)) | 1115 | str(error), |
1063 | 1116 | "Ambigious to debug all hooks and named hooks " | ||
1064 | 1117 | "['*', 'db-relation-changed']") | ||
1065 | 1116 | 1118 | ||
1066 | 1117 | @inlineCallbacks | 1119 | @inlineCallbacks |
1067 | 1118 | def test_enable_debug_requires_sequence(self): | 1120 | def test_enable_debug_requires_sequence(self): |
1068 | @@ -1123,7 +1125,7 @@ | |||
1069 | 1123 | error = yield self.assertFailure( | 1125 | error = yield self.assertFailure( |
1070 | 1124 | unit_state.enable_hook_debug(None), | 1126 | unit_state.enable_hook_debug(None), |
1071 | 1125 | AssertionError) | 1127 | AssertionError) |
1073 | 1126 | self.assertIn("hook names must be a list", str(error)) | 1128 | self.assertEquals(str(error), "Hook names must be a list: got None") |
1074 | 1127 | 1129 | ||
1075 | 1128 | @inlineCallbacks | 1130 | @inlineCallbacks |
1076 | 1129 | def test_enable_named_debug_hook(self): | 1131 | def test_enable_named_debug_hook(self): |
1077 | @@ -1144,8 +1146,10 @@ | |||
1078 | 1144 | """ | 1146 | """ |
1079 | 1145 | unit_state = yield self.get_unit_state() | 1147 | unit_state = yield self.get_unit_state() |
1080 | 1146 | yield unit_state.enable_hook_debug(["*"]) | 1148 | yield unit_state.enable_hook_debug(["*"]) |
1083 | 1147 | yield self.assertFailure(unit_state.enable_hook_debug(["*"]), | 1149 | error = yield self.assertFailure(unit_state.enable_hook_debug(["*"]), |
1084 | 1148 | ServiceUnitDebugAlreadyEnabled) | 1150 | ServiceUnitDebugAlreadyEnabled) |
1085 | 1151 | self.assertEquals( | ||
1086 | 1152 | str(error), "Service unit 'wordpress/0' is already in debug mode.") | ||
1087 | 1149 | 1153 | ||
1088 | 1150 | @inlineCallbacks | 1154 | @inlineCallbacks |
1089 | 1151 | def test_enable_debug_hook_lifetime(self): | 1155 | def test_enable_debug_hook_lifetime(self): |
1090 | 1152 | 1156 | ||
1091 | === modified file 'ensemble/tests/test_errors.py' | |||
1092 | --- ensemble/tests/test_errors.py 2011-08-17 17:26:19 +0000 | |||
1093 | +++ ensemble/tests/test_errors.py 2011-08-26 13:09:07 +0000 | |||
1094 | @@ -23,20 +23,20 @@ | |||
1095 | 23 | 23 | ||
1096 | 24 | def test_FileNotFound(self): | 24 | def test_FileNotFound(self): |
1097 | 25 | error = FileNotFound("/path") | 25 | error = FileNotFound("/path") |
1099 | 26 | self.assertEquals(str(error), "File was not found: /path") | 26 | self.assertEquals(str(error), "File was not found: '/path'") |
1100 | 27 | self.assertIsEnsembleError(error) | 27 | self.assertIsEnsembleError(error) |
1101 | 28 | 28 | ||
1102 | 29 | def test_FileAlreadyExists(self): | 29 | def test_FileAlreadyExists(self): |
1103 | 30 | error = FileAlreadyExists("/path") | 30 | error = FileAlreadyExists("/path") |
1104 | 31 | self.assertEquals(str(error), | 31 | self.assertEquals(str(error), |
1106 | 32 | "File already exists, won't overwrite: /path") | 32 | "File already exists, won't overwrite: '/path'") |
1107 | 33 | self.assertIsEnsembleError(error) | 33 | self.assertIsEnsembleError(error) |
1108 | 34 | 34 | ||
1109 | 35 | def test_InvalidEnsembleHeaderValue(self): | 35 | def test_InvalidEnsembleHeaderValue(self): |
1110 | 36 | error = InvalidEnsembleHeaderValue("/path", "xpctd", "fnd") | 36 | error = InvalidEnsembleHeaderValue("/path", "xpctd", "fnd") |
1111 | 37 | self.assertEquals(str(error), | 37 | self.assertEquals(str(error), |
1112 | 38 | "Expected a YAML file with an 'ensemble: xpctd' " | 38 | "Expected a YAML file with an 'ensemble: xpctd' " |
1114 | 39 | "header, found 'ensemble: fnd': /path") | 39 | "header, found 'ensemble: fnd': '/path'") |
1115 | 40 | self.assertIsEnsembleError(error) | 40 | self.assertIsEnsembleError(error) |
1116 | 41 | 41 | ||
1117 | 42 | def test_NoConnection(self): | 42 | def test_NoConnection(self): |
1118 | @@ -62,12 +62,9 @@ | |||
1119 | 62 | self.assertIsEnsembleError(error) | 62 | self.assertIsEnsembleError(error) |
1120 | 63 | 63 | ||
1121 | 64 | def test_ProviderInteractionError(self): | 64 | def test_ProviderInteractionError(self): |
1123 | 65 | error = ProviderInteractionError(OSError("Bad Stuff")) | 65 | error = ProviderInteractionError("Bad Stuff") |
1124 | 66 | self.assertIsEnsembleError(error) | 66 | self.assertIsEnsembleError(error) |
1129 | 67 | self.assertEquals( | 67 | self.assertEquals(str(error), "Bad Stuff") |
1126 | 68 | str(error), | ||
1127 | 69 | "ProviderError: Interaction with machine provider failed: " | ||
1128 | 70 | "OSError('Bad Stuff',)") | ||
1130 | 71 | 68 | ||
1131 | 72 | def test_CannotTerminateMachine(self): | 69 | def test_CannotTerminateMachine(self): |
1132 | 73 | error = CannotTerminateMachine(0, "environment would be destroyed") | 70 | error = CannotTerminateMachine(0, "environment would be destroyed") |
1133 | @@ -101,11 +98,4 @@ | |||
1134 | 101 | 98 | ||
1135 | 102 | def testEnvironmentPendingWithInfo(self): | 99 | def testEnvironmentPendingWithInfo(self): |
1136 | 103 | error = EnvironmentPending("problem") | 100 | error = EnvironmentPending("problem") |
1145 | 104 | self.assertEquals(str(error), | 101 | self.assertEquals(str(error), "problem") |
1138 | 105 | "Ensemble environment is not accessible: problem") | ||
1139 | 106 | |||
1140 | 107 | def testEnvironmentPendingNoInfo(self): | ||
1141 | 108 | error = EnvironmentPending() | ||
1142 | 109 | self.assertEquals(str(error), | ||
1143 | 110 | "Ensemble environment is not accessible: no " | ||
1144 | 111 | "details available") | ||
1146 | 112 | 102 | ||
1147 | === modified file 'ensemble/unit/tests/test_formula.py' | |||
1148 | --- ensemble/unit/tests/test_formula.py 2011-08-18 21:33:55 +0000 | |||
1149 | +++ ensemble/unit/tests/test_formula.py 2011-08-26 13:09:07 +0000 | |||
1150 | @@ -191,4 +191,5 @@ | |||
1151 | 191 | self.client, "local:mickey-21", formula_directory), | 191 | self.client, "local:mickey-21", formula_directory), |
1152 | 192 | FormulaStateNotFound) | 192 | FormulaStateNotFound) |
1153 | 193 | 193 | ||
1155 | 194 | self.assertIn("Formula 'local:mickey-21' was not found", str(error)) | 194 | self.assertEquals(str(error), |
1156 | 195 | "Formula 'local:mickey-21' was not found") |
Looks generally good, with a few exceptions:
[1]
+ self.name = type(error) .__name_ _ if error else "error"
+ self.action = action or "interacting with provider"
+ self.message = message or str(error)
def __str__(self):
- return "ProviderError: Interaction with machine provider failed: %r" \
- % self.error
+ return "Unexpected %s %s: %s" % (self.name, self.action, self.message)
This sounds somewhat confusing, and quite likely to turn out
in bad error messages. There's no way to make sense, or to
introduce new messages, without looking at the implementation.
Improving on the original error message is good, but please
use something more straightforward here and in the call sites.
[2]
- raise FormulaError(path, "Must be a ZIP-file (%s)." % str(exc))
+ raise FormulaError(path, "must be a zip file (%s)" % exc)
Our error messages start with a capitalized word.
[3]
raise FormulaError(path, yaml\". ") yaml\"" )
- "Archive does not contain required file: "
- "\"metadata.
+ "archive does not contain required file "
+ "\"metadata.
Same thing.
[4]
- @param master: if True, machine will initialize the ensemble admin
+ `master` if True, machine will initialize the ensemble admin
and run a provisioning agent.
I suspect this is the first time I see this notation, and our overall comment
style is starting to feel like a soup. We have at least three different kinds
of comments, besides the broken ones. Please revert the changes to parameter
names and raising/returning specifically so that this branch can be merged
and let's talk about styling in a different thread.