Merge lp:~abentley/charmworld/remove-store-ingest-job into lp:~juju-jitsu/charmworld/trunk
- remove-store-ingest-job
- Merge into trunk
Proposed by
Aaron Bentley
Status: | Merged |
---|---|
Approved by: | Curtis Hovey |
Approved revision: | 293 |
Merged at revision: | 283 |
Proposed branch: | lp:~abentley/charmworld/remove-store-ingest-job |
Merge into: | lp:~juju-jitsu/charmworld/trunk |
Diff against target: |
664 lines (+204/-226) 4 files modified
charmworld/jobs/ingest.py (+142/-171) charmworld/jobs/tests/test_bzr.py (+24/-26) charmworld/jobs/tests/test_proof.py (+13/-10) charmworld/jobs/tests/test_store.py (+25/-19) |
To merge this branch: | bzr merge lp:~abentley/charmworld/remove-store-ingest-job |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Curtis Hovey (community) | code | Approve | |
Review via email: mp+171162@code.launchpad.net |
Commit message
Remove remaining sub-Jobs from ingest.
Description of the change
This branch completes the work for bug #1190015: ingest is really a set of functions that are run consecutively, but is structured as a list of jobs.
It removes StoreIngestJob, ChangelogIngestJob and ProofIngestJob. The remaining code has some room for improvement, espeicially some very short functions, but this can be addressed piecemeal.
IngestJob and DBIngestJob are retained because they are parents of the remaining job, UpdateCharmJob.
As a driveby, the 'retry' parameter was removed from do_bzr_update, since it was never used.
To post a comment you must log in.
Preview Diff
[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1 | === modified file 'charmworld/jobs/ingest.py' | |||
2 | --- charmworld/jobs/ingest.py 2013-06-24 15:45:29 +0000 | |||
3 | +++ charmworld/jobs/ingest.py 2013-06-24 19:49:29 +0000 | |||
4 | @@ -88,14 +88,14 @@ | |||
5 | 88 | self.db = db | 88 | self.db = db |
6 | 89 | 89 | ||
7 | 90 | 90 | ||
9 | 91 | def do_bzr_update(charm_data, db, fs, log, root_dir=None, retry=True): | 91 | def do_bzr_update(charm_data, db, fs, log, root_dir=None): |
10 | 92 | """Fetch a branch from bzr, and augment charm data.""" | 92 | """Fetch a branch from bzr, and augment charm data.""" |
11 | 93 | if not root_dir: | 93 | if not root_dir: |
12 | 94 | root_dir = CHARM_DIR | 94 | root_dir = CHARM_DIR |
13 | 95 | root_dir = root_dir | 95 | root_dir = root_dir |
14 | 96 | if not os.path.exists(root_dir): | 96 | if not os.path.exists(root_dir): |
15 | 97 | os.makedirs(root_dir) | 97 | os.makedirs(root_dir) |
17 | 98 | update_charm_files(root_dir, fs, charm_data, log, retry) | 98 | update_charm_files(root_dir, fs, charm_data, log) |
18 | 99 | 99 | ||
19 | 100 | 100 | ||
20 | 101 | def update_charm_branch(root_dir, fs, charm_data, branch_dir, log, | 101 | def update_charm_branch(root_dir, fs, charm_data, branch_dir, log, |
21 | @@ -153,7 +153,7 @@ | |||
22 | 153 | return charm_data | 153 | return charm_data |
23 | 154 | 154 | ||
24 | 155 | 155 | ||
26 | 156 | def update_charm_files(root_dir, fs, charm_data, log, retry): | 156 | def update_charm_files(root_dir, fs, charm_data, log, retry=True): |
27 | 157 | if charm_data['branch_deleted']: | 157 | if charm_data['branch_deleted']: |
28 | 158 | return | 158 | return |
29 | 159 | branch_dir = os.path.abspath( | 159 | branch_dir = os.path.abspath( |
30 | @@ -225,13 +225,10 @@ | |||
31 | 225 | fs = getfs(db) | 225 | fs = getfs(db) |
32 | 226 | try: | 226 | try: |
33 | 227 | do_bzr_update(charm_data, db, fs, log) | 227 | do_bzr_update(charm_data, db, fs, log) |
38 | 228 | if not run_job(StoreIngestJob(), charm_data): | 228 | update_from_store(charm_data, CharmStore(), log) |
39 | 229 | return False | 229 | update_proof_data(charm_data, log) |
36 | 230 | if not run_job(ProofIngestJob(), charm_data): | ||
37 | 231 | return False | ||
40 | 232 | update_jenkins_data(db, charm_data, log) | 230 | update_jenkins_data(db, charm_data, log) |
43 | 233 | if not run_job(ChangelogIngestJob(), charm_data): | 231 | update_from_revisions(charm_data) |
42 | 234 | return False | ||
44 | 235 | update_date_created(charm_data, log) | 232 | update_date_created(charm_data, log) |
45 | 236 | scan_charm(charm_data, db, fs, log) | 233 | scan_charm(charm_data, db, fs, log) |
46 | 237 | except Exception as e: | 234 | except Exception as e: |
47 | @@ -258,78 +255,69 @@ | |||
48 | 258 | CharmSource(self.db, index_client).save(charm_data) | 255 | CharmSource(self.db, index_client).save(charm_data) |
49 | 259 | 256 | ||
50 | 260 | 257 | ||
116 | 261 | class ChangelogIngestJob(IngestJob): | 258 | def _rev_info(r, branch): |
117 | 262 | 259 | d = { | |
118 | 263 | name = 'changelog' | 260 | 'authors': r.get_apparent_authors(), |
119 | 264 | 261 | "revno": branch.revision_id_to_revno(r.revision_id), | |
120 | 265 | def __init__(self, limit=10, since=None): | 262 | "committer": r.committer, |
121 | 266 | self.limit = limit | 263 | "created": r.timestamp, |
122 | 267 | self.since = since | 264 | "message": r.message |
123 | 268 | 265 | } | |
124 | 269 | def setup(self): | 266 | return d |
125 | 270 | super(ChangelogIngestJob, self).setup() | 267 | |
126 | 271 | if self.since is None: | 268 | |
127 | 272 | days_of_revisions = settings.get('days_of_revisions') | 269 | def update_from_revisions(charm_data, limit=10, since=None): |
128 | 273 | if days_of_revisions is not None: | 270 | if charm_data['branch_deleted']: |
129 | 274 | cutoff = datetime.utcnow() - timedelta(int(days_of_revisions)) | 271 | return |
130 | 275 | self.since = calendar.timegm(cutoff.timetuple()) | 272 | if since is None: |
131 | 276 | 273 | days_of_revisions = settings.get('days_of_revisions') | |
132 | 277 | @staticmethod | 274 | if days_of_revisions is not None: |
133 | 278 | def _rev_info(r, branch): | 275 | cutoff = datetime.utcnow() - timedelta(int(days_of_revisions)) |
134 | 279 | d = { | 276 | since = calendar.timegm(cutoff.timetuple()) |
135 | 280 | 'authors': r.get_apparent_authors(), | 277 | branch_dir = charm_data["branch_dir"] |
136 | 281 | "revno": branch.revision_id_to_revno(r.revision_id), | 278 | charm_data.update(get_changes(branch_dir, limit, since)) |
137 | 282 | "committer": r.committer, | 279 | |
138 | 283 | "created": r.timestamp, | 280 | |
139 | 284 | "message": r.message | 281 | def get_changes(branch_dir, limit, since): |
140 | 285 | } | 282 | charm_data = {} |
141 | 286 | return d | 283 | branch = Branch.open(branch_dir) |
142 | 287 | 284 | branch.lock_read() | |
143 | 288 | def run(self, charm_data): | 285 | try: |
144 | 289 | if charm_data['branch_deleted']: | 286 | revisions = get_revisions(branch, limit, since) |
145 | 290 | return | 287 | charm_data["changes"] = changes = [] |
146 | 291 | branch_dir = charm_data["branch_dir"] | 288 | for r in revisions: |
147 | 292 | charm_data.update(self.get_changes(branch_dir)) | 289 | changes.append(_rev_info(r, branch)) |
148 | 293 | 290 | if len(revisions) == 0: | |
149 | 294 | def get_changes(self, branch_dir): | 291 | last_change = None |
150 | 295 | charm_data = {} | 292 | first_change = None |
151 | 296 | branch = Branch.open(branch_dir) | 293 | else: |
152 | 297 | branch.lock_read() | 294 | last_change = changes[0] |
153 | 298 | try: | 295 | first = branch.repository.get_revision(branch.get_rev_id(1)) |
154 | 299 | revisions = self.get_revisions(branch) | 296 | first_change = _rev_info(first, branch) |
155 | 300 | charm_data["changes"] = changes = [] | 297 | charm_data.update({ |
156 | 301 | for r in revisions: | 298 | 'last_change': last_change, |
157 | 302 | changes.append(self._rev_info(r, branch)) | 299 | 'first_change': first_change, |
158 | 303 | if len(revisions) == 0: | 300 | }) |
159 | 304 | last_change = None | 301 | return charm_data |
160 | 305 | first_change = None | 302 | finally: |
161 | 306 | else: | 303 | branch.unlock() |
162 | 307 | last_change = changes[0] | 304 | |
163 | 308 | first = branch.repository.get_revision(branch.get_rev_id(1)) | 305 | |
164 | 309 | first_change = self._rev_info(first, branch) | 306 | def get_revisions(branch, limit, since): |
165 | 310 | charm_data.update({ | 307 | # We only want the last 10 changes, in descending order. |
166 | 311 | 'last_change': last_change, | 308 | graph = branch.repository.get_graph() |
167 | 312 | 'first_change': first_change, | 309 | cur_rev_id = branch.last_revision() |
168 | 313 | }) | 310 | ancestry_iter = graph.iter_lefthand_ancestry(cur_rev_id) |
169 | 314 | return charm_data | 311 | revs = [] |
170 | 315 | finally: | 312 | for num, revision_id in enumerate(ancestry_iter): |
171 | 316 | branch.unlock() | 313 | if revision_id == NULL_REVISION: |
172 | 317 | 314 | break | |
173 | 318 | def get_revisions(self, branch): | 315 | revision = branch.repository.get_revision(revision_id) |
174 | 319 | # We only want the last 10 changes, in descending order. | 316 | if num >= limit: |
175 | 320 | graph = branch.repository.get_graph() | 317 | if since is None or revision.timestamp < since: |
111 | 321 | cur_rev_id = branch.last_revision() | ||
112 | 322 | ancestry_iter = graph.iter_lefthand_ancestry(cur_rev_id) | ||
113 | 323 | revs = [] | ||
114 | 324 | for num, revision_id in enumerate(ancestry_iter): | ||
115 | 325 | if revision_id == NULL_REVISION: | ||
176 | 326 | break | 318 | break |
183 | 327 | revision = branch.repository.get_revision(revision_id) | 319 | revs.append(revision) |
184 | 328 | if num >= self.limit: | 320 | return revs |
179 | 329 | if self.since is None or revision.timestamp < self.since: | ||
180 | 330 | break | ||
181 | 331 | revs.append(revision) | ||
182 | 332 | return revs | ||
185 | 333 | 321 | ||
186 | 334 | 322 | ||
187 | 335 | def update_jenkins_data(db, charm, log): | 323 | def update_jenkins_data(db, charm, log): |
188 | @@ -414,66 +402,62 @@ | |||
189 | 414 | return (result_id, result['result']) | 402 | return (result_id, result['result']) |
190 | 415 | 403 | ||
191 | 416 | 404 | ||
252 | 417 | class ProofIngestJob(IngestJob): | 405 | @contextlib.contextmanager |
253 | 418 | 406 | def get_proof_lib(new_path): | |
254 | 419 | name = 'proof' | 407 | if new_path not in sys.path: |
255 | 420 | 408 | sys.path.append(new_path) | |
256 | 421 | @contextlib.contextmanager | 409 | try: |
257 | 422 | def _get_proof_lib(self, new_path): | 410 | import lib.proof as prooflib |
258 | 423 | if new_path not in sys.path: | 411 | yield prooflib |
259 | 424 | sys.path.append(new_path) | 412 | except ImportError: |
260 | 425 | try: | 413 | yield None |
261 | 426 | import lib.proof as prooflib | 414 | finally: |
262 | 427 | yield prooflib | 415 | if new_path in sys.path: |
263 | 428 | except ImportError: | 416 | sys.path.remove(new_path) |
264 | 429 | yield None | 417 | |
265 | 430 | finally: | 418 | |
266 | 431 | if new_path in sys.path: | 419 | def get_proofer(log, proof_path=None): |
267 | 432 | sys.path.remove(new_path) | 420 | proofer = None |
268 | 433 | 421 | if not proof_path: | |
269 | 434 | def setup(self, proofer=None, proof_path=None): | 422 | proof_path = CHARM_PROOF_PATH |
270 | 435 | if not proofer: | 423 | |
271 | 436 | proofer = self.get_proofer(proof_path) | 424 | # Use config.CHARM_PROOF_PATH for testing. Monkeypatching is scary. |
272 | 437 | self.proofer = proofer | 425 | if not os.path.isdir(proof_path): |
273 | 438 | 426 | err_msg = ("proof error before processing began: could not find " | |
274 | 439 | def get_proofer(self, proof_path=None): | 427 | "charm proof path.") |
275 | 440 | proofer = None | 428 | log.exception(err_msg) |
276 | 441 | if not proof_path: | 429 | log.exception("CHARM_PROOF_PATH: %s", proof_path) |
277 | 442 | proof_path = CHARM_PROOF_PATH | 430 | with get_proof_lib(proof_path) as prooflib: |
278 | 443 | 431 | if not prooflib: | |
279 | 444 | # Use config.CHARM_PROOF_PATH for testing. Monkeypatching is scary. | 432 | err_msg = ("proof error before processing began: could not " |
280 | 445 | if not os.path.isdir(proof_path): | 433 | "import charm proof lib.") |
281 | 446 | err_msg = ("proof error before processing began: could not find " | 434 | log.exception(err_msg) |
282 | 447 | "charm proof path.") | 435 | log.exception( |
283 | 448 | self.log.exception(err_msg) | 436 | "CHARM_PROOF_PATH: %s", proof_path) |
284 | 449 | self.log.exception("CHARM_PROOF_PATH: %s", proof_path) | 437 | else: |
285 | 450 | with self._get_proof_lib(proof_path) as prooflib: | 438 | proofer = prooflib.run |
286 | 451 | if not prooflib: | 439 | return proofer |
287 | 452 | err_msg = ("proof error before processing began: could not " | 440 | |
288 | 453 | "import charm proof lib.") | 441 | |
289 | 454 | self.log.exception(err_msg) | 442 | def update_proof_data(charm, log, _proofer=None): |
290 | 455 | self.log.exception( | 443 | proofer = _proofer |
291 | 456 | "CHARM_PROOF_PATH: %s", proof_path) | 444 | if proofer is None: |
292 | 457 | else: | 445 | proofer = get_proofer(log) |
293 | 458 | proofer = prooflib.run | 446 | if not proofer: |
294 | 459 | return proofer | 447 | log.exception("proof aborted.") |
295 | 460 | 448 | raise Exception("No proofer") | |
296 | 461 | def run(self, charm): | 449 | if charm['branch_deleted']: |
297 | 462 | if not self.proofer: | 450 | return |
298 | 463 | self.log.exception("proof aborted.") | 451 | proof = {} |
299 | 464 | raise Exception("No proofer") | 452 | lint, exit_code = proofer(charm['branch_dir']) |
300 | 465 | if charm['branch_deleted']: | 453 | for line in lint: |
301 | 466 | return | 454 | if not ':' in line: |
302 | 467 | proof = {} | 455 | continue |
303 | 468 | lint, exit_code = self.proofer(charm['branch_dir']) | 456 | level, msg = line.split(':', 1) |
304 | 469 | for line in lint: | 457 | if level == "W" and 'name' in msg: |
305 | 470 | if not ':' in line: | 458 | continue |
306 | 471 | continue | 459 | proof.setdefault(level.lower(), []).append(msg) |
307 | 472 | level, msg = line.split(':', 1) | 460 | charm['proof'] = proof |
248 | 473 | if level == "W" and 'name' in msg: | ||
249 | 474 | continue | ||
250 | 475 | proof.setdefault(level.lower(), []).append(msg) | ||
251 | 476 | charm['proof'] = proof | ||
308 | 477 | 461 | ||
309 | 478 | 462 | ||
310 | 479 | def scan_charm(charm_data, db, fs, log): | 463 | def scan_charm(charm_data, db, fs, log): |
311 | @@ -639,37 +623,24 @@ | |||
312 | 639 | charm['date_created'] = timestamp(date_created.replace(microsecond=0)) | 623 | charm['date_created'] = timestamp(date_created.replace(microsecond=0)) |
313 | 640 | 624 | ||
314 | 641 | 625 | ||
346 | 642 | class StoreIngestJob(IngestJob): | 626 | def update_from_store(charm, store, log): |
347 | 643 | 627 | old_address = None | |
348 | 644 | name = 'store' | 628 | for address in addresses(charm): |
349 | 645 | 629 | if old_address is not None: | |
350 | 646 | def __init__(self, store=None): | 630 | log.info("rechecking %s with %s", old_address, address) |
351 | 647 | super(StoreIngestJob, self).__init__() | 631 | old_address = address |
352 | 648 | if store is None: | 632 | data = store.get_charm_info(address) |
353 | 649 | store = CharmStore() | 633 | if 'errors' not in data and 'warnings' not in data: |
354 | 650 | self.store = store | 634 | break |
355 | 651 | 635 | ||
356 | 652 | def run(self, charm): | 636 | if 'errors' in data or 'warnings' in data: |
357 | 653 | old_address = None | 637 | log.warning("store error on %s %s" % (address, data)) |
358 | 654 | for address in addresses(charm): | 638 | |
359 | 655 | if old_address is not None: | 639 | data["store_checked"] = datetime.now().ctime() |
360 | 656 | self.log.info("rechecking %s with %s", old_address, address) | 640 | |
361 | 657 | old_address = address | 641 | charm['store_data'] = data |
362 | 658 | data = self._store_get(address) | 642 | charm['store_url'] = make_store_url(data['revision'], address) |
363 | 659 | if 'errors' not in data and 'warnings' not in data: | 643 | update_download_count(store, charm) |
333 | 660 | break | ||
334 | 661 | |||
335 | 662 | if 'errors' in data or 'warnings' in data: | ||
336 | 663 | self.log.warning("store error on %s %s" % (address, data)) | ||
337 | 664 | |||
338 | 665 | data["store_checked"] = datetime.now().ctime() | ||
339 | 666 | |||
340 | 667 | charm['store_data'] = data | ||
341 | 668 | charm['store_url'] = make_store_url(data['revision'], address) | ||
342 | 669 | update_download_count(self.store, charm) | ||
343 | 670 | |||
344 | 671 | def _store_get(self, address): | ||
345 | 672 | return CharmStore().get_charm_info(address) | ||
364 | 673 | 644 | ||
365 | 674 | 645 | ||
366 | 675 | # XXX j.c.sackett Jan 31 2013 Bug:1111708 scan_repo is swapped for | 646 | # XXX j.c.sackett Jan 31 2013 Bug:1111708 scan_repo is swapped for |
367 | 676 | 647 | ||
368 | === modified file 'charmworld/jobs/tests/test_bzr.py' | |||
369 | --- charmworld/jobs/tests/test_bzr.py 2013-06-24 15:45:29 +0000 | |||
370 | +++ charmworld/jobs/tests/test_bzr.py 2013-06-24 19:49:29 +0000 | |||
371 | @@ -6,15 +6,20 @@ | |||
372 | 6 | from logging import getLogger | 6 | from logging import getLogger |
373 | 7 | import os | 7 | import os |
374 | 8 | import shutil | 8 | import shutil |
375 | 9 | import time | ||
376 | 9 | 10 | ||
377 | 10 | from bzrlib.bzrdir import BzrDir | 11 | from bzrlib.bzrdir import BzrDir |
378 | 11 | from mock import patch | 12 | from mock import patch |
379 | 12 | 13 | ||
380 | 13 | 14 | ||
381 | 15 | from charmworld.jobs import ingest | ||
382 | 14 | from charmworld.jobs.ingest import ( | 16 | from charmworld.jobs.ingest import ( |
383 | 15 | add_files, | 17 | add_files, |
384 | 16 | do_bzr_update, | 18 | do_bzr_update, |
386 | 17 | ChangelogIngestJob, | 19 | get_changes, |
387 | 20 | get_revisions, | ||
388 | 21 | _rev_info, | ||
389 | 22 | update_from_revisions, | ||
390 | 18 | ) | 23 | ) |
391 | 19 | from charmworld.models import getfs | 24 | from charmworld.models import getfs |
392 | 20 | from charmworld.testing import ( | 25 | from charmworld.testing import ( |
393 | @@ -169,24 +174,23 @@ | |||
394 | 169 | 174 | ||
395 | 170 | def test_get_revisions(self): | 175 | def test_get_revisions(self): |
396 | 171 | tree = self.make_locked_tree() | 176 | tree = self.make_locked_tree() |
399 | 172 | job = ChangelogIngestJob() | 177 | revisions = get_revisions(tree.branch, 10, 0) |
398 | 173 | revisions = job.get_revisions(tree.branch) | ||
400 | 174 | self.assertEqual([], revisions) | 178 | self.assertEqual([], revisions) |
401 | 175 | for x in range(9): | 179 | for x in range(9): |
402 | 176 | self.num_commit(tree, x) | 180 | self.num_commit(tree, x) |
404 | 177 | revisions = job.get_revisions(tree.branch) | 181 | revisions = get_revisions(tree.branch, 10, 0) |
405 | 178 | self.assertEqual(["revision %d" % num for num in range(8, -1, -1)], | 182 | self.assertEqual(["revision %d" % num for num in range(8, -1, -1)], |
406 | 179 | [revision.message for revision in revisions]) | 183 | [revision.message for revision in revisions]) |
407 | 180 | self.num_commit(tree, 9) | 184 | self.num_commit(tree, 9) |
409 | 181 | revisions = job.get_revisions(tree.branch) | 185 | revisions = get_revisions(tree.branch, 10, time.time()) |
410 | 182 | self.assertEqual(["revision %d" % num for num in range(9, -1, -1)], | 186 | self.assertEqual(["revision %d" % num for num in range(9, -1, -1)], |
411 | 183 | [revision.message for revision in revisions]) | 187 | [revision.message for revision in revisions]) |
412 | 184 | self.num_commit(tree, 10) | 188 | self.num_commit(tree, 10) |
414 | 185 | revisions = job.get_revisions(tree.branch) | 189 | revisions = get_revisions(tree.branch, 10, time.time()) |
415 | 186 | self.assertEqual(["revision %d" % num for num in range(10, 0, -1)], | 190 | self.assertEqual(["revision %d" % num for num in range(10, 0, -1)], |
416 | 187 | [revision.message for revision in revisions]) | 191 | [revision.message for revision in revisions]) |
417 | 188 | self.num_commit(tree, 11) | 192 | self.num_commit(tree, 11) |
419 | 189 | revisions = job.get_revisions(tree.branch) | 193 | revisions = get_revisions(tree.branch, 10, time.time()) |
420 | 190 | self.assertEqual(["revision %d" % num for num in range(11, 1, -1)], | 194 | self.assertEqual(["revision %d" % num for num in range(11, 1, -1)], |
421 | 191 | [revision.message for revision in revisions]) | 195 | [revision.message for revision in revisions]) |
422 | 192 | 196 | ||
423 | @@ -194,9 +198,9 @@ | |||
424 | 194 | tree = self.make_locked_tree() | 198 | tree = self.make_locked_tree() |
425 | 195 | for x in range(15): | 199 | for x in range(15): |
426 | 196 | self.num_commit(tree, x) | 200 | self.num_commit(tree, x) |
428 | 197 | revisions = ChangelogIngestJob(limit=5).get_revisions(tree.branch) | 201 | revisions = get_revisions(tree.branch, 5, time.time()) |
429 | 198 | self.assertEqual(5, len(revisions)) | 202 | self.assertEqual(5, len(revisions)) |
431 | 199 | revisions = ChangelogIngestJob(limit=20).get_revisions(tree.branch) | 203 | revisions = get_revisions(tree.branch, 20, time.time()) |
432 | 200 | self.assertEqual(15, len(revisions)) | 204 | self.assertEqual(15, len(revisions)) |
433 | 201 | 205 | ||
434 | 202 | def test_get_revisions_honours_since(self): | 206 | def test_get_revisions_honours_since(self): |
435 | @@ -204,11 +208,9 @@ | |||
436 | 204 | tree.commit('', timestamp=0, committer='') | 208 | tree.commit('', timestamp=0, committer='') |
437 | 205 | tree.commit('', timestamp=1, committer='') | 209 | tree.commit('', timestamp=1, committer='') |
438 | 206 | tree.commit('', timestamp=2, committer='') | 210 | tree.commit('', timestamp=2, committer='') |
441 | 207 | job = ChangelogIngestJob(since=0, limit=0) | 211 | revisions = get_revisions(tree.branch, 0, 0) |
440 | 208 | revisions = job.get_revisions(tree.branch) | ||
442 | 209 | self.assertEqual(3, len(revisions)) | 212 | self.assertEqual(3, len(revisions)) |
445 | 210 | job = ChangelogIngestJob(since=1, limit=0) | 213 | revisions = get_revisions(tree.branch, 0, 1) |
444 | 211 | revisions = job.get_revisions(tree.branch) | ||
446 | 212 | self.assertEqual(2, len(revisions)) | 214 | self.assertEqual(2, len(revisions)) |
447 | 213 | 215 | ||
448 | 214 | def test_limit_overrides_since(self): | 216 | def test_limit_overrides_since(self): |
449 | @@ -216,8 +218,7 @@ | |||
450 | 216 | tree.commit('', timestamp=0, committer='') | 218 | tree.commit('', timestamp=0, committer='') |
451 | 217 | tree.commit('', timestamp=1, committer='') | 219 | tree.commit('', timestamp=1, committer='') |
452 | 218 | tree.commit('', timestamp=2, committer='') | 220 | tree.commit('', timestamp=2, committer='') |
455 | 219 | job = ChangelogIngestJob(since=1, limit=3) | 221 | revisions = get_revisions(tree.branch, since=1, limit=3) |
454 | 220 | revisions = job.get_revisions(tree.branch) | ||
456 | 221 | self.assertEqual(3, len(revisions)) | 222 | self.assertEqual(3, len(revisions)) |
457 | 222 | 223 | ||
458 | 223 | def test_rev_info(self): | 224 | def test_rev_info(self): |
459 | @@ -225,7 +226,7 @@ | |||
460 | 225 | revision_id = tree.commit('message1', committer=JRH) | 226 | revision_id = tree.commit('message1', committer=JRH) |
461 | 226 | rev = tree.branch.repository.get_revision(revision_id) | 227 | rev = tree.branch.repository.get_revision(revision_id) |
462 | 227 | rev.timestamp = 34 | 228 | rev.timestamp = 34 |
464 | 228 | rev_info = ChangelogIngestJob._rev_info(rev, tree.branch) | 229 | rev_info = _rev_info(rev, tree.branch) |
465 | 229 | self.assertEqual({ | 230 | self.assertEqual({ |
466 | 230 | 'authors': [JRH], | 231 | 'authors': [JRH], |
467 | 231 | 'committer': JRH, | 232 | 'committer': JRH, |
468 | @@ -236,8 +237,7 @@ | |||
469 | 236 | 237 | ||
470 | 237 | @staticmethod | 238 | @staticmethod |
471 | 238 | def get_rev_info(branch, revision_id): | 239 | def get_rev_info(branch, revision_id): |
474 | 239 | rev = branch.repository.get_revision(revision_id) | 240 | return _rev_info(branch.repository.get_revision(revision_id), branch) |
473 | 240 | return ChangelogIngestJob._rev_info(rev, branch) | ||
475 | 241 | 241 | ||
476 | 242 | def test_rev_info_supplied_authors(self): | 242 | def test_rev_info_supplied_authors(self): |
477 | 243 | tree = self.make_locked_tree() | 243 | tree = self.make_locked_tree() |
478 | @@ -250,15 +250,14 @@ | |||
479 | 250 | def test_get_changes(self): | 250 | def test_get_changes(self): |
480 | 251 | tree = self.make_locked_tree() | 251 | tree = self.make_locked_tree() |
481 | 252 | first_id = self.num_commit(tree, 1) | 252 | first_id = self.num_commit(tree, 1) |
484 | 253 | job = ChangelogIngestJob() | 253 | charm_data = get_changes('tree', 10, time.time()) |
483 | 254 | charm_data = job.get_changes('tree') | ||
485 | 255 | self.assertItemsEqual(['changes', 'first_change', 'last_change'], | 254 | self.assertItemsEqual(['changes', 'first_change', 'last_change'], |
486 | 256 | charm_data.keys()) | 255 | charm_data.keys()) |
487 | 257 | self.assertEqual(charm_data['first_change'], charm_data['last_change']) | 256 | self.assertEqual(charm_data['first_change'], charm_data['last_change']) |
488 | 258 | self.assertEqual([charm_data['first_change']], charm_data['changes']) | 257 | self.assertEqual([charm_data['first_change']], charm_data['changes']) |
489 | 259 | for num in range(1, 11): | 258 | for num in range(1, 11): |
490 | 260 | self.num_commit(tree, 1) | 259 | self.num_commit(tree, 1) |
492 | 261 | charm_data = job.get_changes('tree') | 260 | charm_data = get_changes('tree', 10, time.time()) |
493 | 262 | self.assertNotEqual(charm_data['first_change'], | 261 | self.assertNotEqual(charm_data['first_change'], |
494 | 263 | charm_data['last_change']) | 262 | charm_data['last_change']) |
495 | 264 | self.assertEqual(charm_data['last_change'], charm_data['changes'][0]) | 263 | self.assertEqual(charm_data['last_change'], charm_data['changes'][0]) |
496 | @@ -269,16 +268,15 @@ | |||
497 | 269 | 268 | ||
498 | 270 | def test_get_changes_no_revisions(self): | 269 | def test_get_changes_no_revisions(self): |
499 | 271 | self.make_locked_tree() | 270 | self.make_locked_tree() |
501 | 272 | charm_data = ChangelogIngestJob().get_changes('tree') | 271 | charm_data = get_changes('tree', 10, time.time()) |
502 | 273 | self.assertEqual([], charm_data['changes']) | 272 | self.assertEqual([], charm_data['changes']) |
503 | 274 | self.assertIs(None, charm_data['first_change']) | 273 | self.assertIs(None, charm_data['first_change']) |
504 | 275 | self.assertIs(None, charm_data['last_change']) | 274 | self.assertIs(None, charm_data['last_change']) |
505 | 276 | 275 | ||
506 | 277 | def test_branch_deleted(self): | 276 | def test_branch_deleted(self): |
508 | 278 | # ChangelogIngestJob does not do anything if the Launchpad branch | 277 | # update_from_revisions does not do anything if the Launchpad branch |
509 | 279 | # of a charm is deleted. | 278 | # of a charm is deleted. |
510 | 280 | job = ChangelogIngestJob() | ||
511 | 281 | charm = factory.get_charm_json(branch_deleted=True) | 279 | charm = factory.get_charm_json(branch_deleted=True) |
514 | 282 | with patch.object(job, 'get_changes') as mock: | 280 | with patch.object(ingest, 'get_changes') as mock: |
515 | 283 | job.run(charm) | 281 | update_from_revisions(charm) |
516 | 284 | self.assertFalse(mock.called) | 282 | self.assertFalse(mock.called) |
517 | 285 | 283 | ||
518 | === modified file 'charmworld/jobs/tests/test_proof.py' | |||
519 | --- charmworld/jobs/tests/test_proof.py 2013-06-07 12:36:30 +0000 | |||
520 | +++ charmworld/jobs/tests/test_proof.py 2013-06-24 19:49:29 +0000 | |||
521 | @@ -1,9 +1,14 @@ | |||
522 | 1 | # Copyright 2012, 2013 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2012, 2013 Canonical Ltd. This software is licensed under the |
523 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
524 | 3 | 3 | ||
528 | 4 | from mock import patch | 4 | from logging import getLogger |
529 | 5 | 5 | ||
530 | 6 | from charmworld.jobs.ingest import ProofIngestJob | 6 | from mock import MagicMock |
531 | 7 | |||
532 | 8 | from charmworld.jobs.ingest import ( | ||
533 | 9 | get_proofer, | ||
534 | 10 | update_proof_data, | ||
535 | 11 | ) | ||
536 | 7 | from charmworld.testing import factory | 12 | from charmworld.testing import factory |
537 | 8 | from charmworld.testing import JobTestBase | 13 | from charmworld.testing import JobTestBase |
538 | 9 | 14 | ||
539 | @@ -14,9 +19,8 @@ | |||
540 | 14 | # If the charm-proof library cannot be found, an error is logged | 19 | # If the charm-proof library cannot be found, an error is logged |
541 | 15 | # and the proof loop aborted. | 20 | # and the proof loop aborted. |
542 | 16 | nonexistant_path = '/does/not/exist/' | 21 | nonexistant_path = '/does/not/exist/' |
543 | 17 | job = ProofIngestJob() | ||
544 | 18 | handler = self.get_handler('charm.proof') | 22 | handler = self.get_handler('charm.proof') |
546 | 19 | job.setup(proof_path=nonexistant_path) | 23 | get_proofer(getLogger('charm.proof'), nonexistant_path) |
547 | 20 | log_messages = [record.getMessage() for record in handler.buffer] | 24 | log_messages = [record.getMessage() for record in handler.buffer] |
548 | 21 | err = ("proof error before processing began: " | 25 | err = ("proof error before processing began: " |
549 | 22 | "could not find charm proof path.") | 26 | "could not find charm proof path.") |
550 | @@ -26,9 +30,8 @@ | |||
551 | 26 | def test_no_proof_for_deleted_branch(self): | 30 | def test_no_proof_for_deleted_branch(self): |
552 | 27 | # If the branch of a charm is deleted from Launchpad, the proof | 31 | # If the branch of a charm is deleted from Launchpad, the proof |
553 | 28 | # job does nothing. | 32 | # job does nothing. |
556 | 29 | job = ProofIngestJob() | 33 | self.proofer = get_proofer(getLogger('charm.proof')) |
555 | 30 | job.setup() | ||
557 | 31 | charm = factory.get_charm_json(branch_deleted=True) | 34 | charm = factory.get_charm_json(branch_deleted=True) |
561 | 32 | with patch.object(job, 'proofer') as mock: | 35 | proofer = MagicMock() |
562 | 33 | job.run(charm) | 36 | update_proof_data(charm, getLogger('charm.proof'), proofer) |
563 | 34 | self.assertFalse(mock.called) | 37 | self.assertFalse(proofer.called) |
564 | 35 | 38 | ||
565 | === modified file 'charmworld/jobs/tests/test_store.py' | |||
566 | --- charmworld/jobs/tests/test_store.py 2013-05-27 12:45:51 +0000 | |||
567 | +++ charmworld/jobs/tests/test_store.py 2013-06-24 19:49:29 +0000 | |||
568 | @@ -1,17 +1,25 @@ | |||
569 | 1 | # Copyright 2012, 2013 Canonical Ltd. This software is licensed under the | 1 | # Copyright 2012, 2013 Canonical Ltd. This software is licensed under the |
570 | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). | 2 | # GNU Affero General Public License version 3 (see the file LICENSE). |
571 | 3 | 3 | ||
572 | 4 | from logging import getLogger | ||
573 | 4 | from mock import patch | 5 | from mock import patch |
574 | 5 | from requests import Response | 6 | from requests import Response |
575 | 6 | 7 | ||
578 | 7 | from charmworld.jobs.ingest import StoreIngestJob | 8 | from charmworld.charmstore import CharmStore |
579 | 8 | from charmworld.jobs.ingest import addresses | 9 | from charmworld.jobs.ingest import ( |
580 | 10 | addresses, | ||
581 | 11 | update_from_store, | ||
582 | 12 | ) | ||
583 | 9 | from charmworld.testing import factory | 13 | from charmworld.testing import factory |
584 | 10 | from charmworld.testing import JobTestBase | 14 | from charmworld.testing import JobTestBase |
585 | 11 | 15 | ||
586 | 12 | 16 | ||
587 | 13 | class TestStoreJob(JobTestBase): | 17 | class TestStoreJob(JobTestBase): |
588 | 14 | 18 | ||
589 | 19 | def setUp(self): | ||
590 | 20 | super(TestStoreJob, self).setUp() | ||
591 | 21 | self.log = getLogger('charm.store') | ||
592 | 22 | |||
593 | 15 | def _mock_data(self, warning=False, error=False): | 23 | def _mock_data(self, warning=False, error=False): |
594 | 16 | data = {'revision': 1} | 24 | data = {'revision': 1} |
595 | 17 | if warning: | 25 | if warning: |
596 | @@ -20,7 +28,7 @@ | |||
597 | 20 | data['errors'] = 'Error forced by mock.' | 28 | data['errors'] = 'Error forced by mock.' |
598 | 21 | return data | 29 | return data |
599 | 22 | 30 | ||
601 | 23 | def falsify_session(self, job, content='[[0]]'): | 31 | def falsify_session(self, store, content='[[0]]'): |
602 | 24 | 32 | ||
603 | 25 | class FakeSession: | 33 | class FakeSession: |
604 | 26 | 34 | ||
605 | @@ -30,17 +38,16 @@ | |||
606 | 30 | response._content = content | 38 | response._content = content |
607 | 31 | return response | 39 | return response |
608 | 32 | 40 | ||
610 | 33 | job.store.session = FakeSession | 41 | store.session = FakeSession |
611 | 34 | 42 | ||
612 | 35 | def test_error_handling_error(self): | 43 | def test_error_handling_error(self): |
613 | 36 | handler = self.get_handler('charm.store') | 44 | handler = self.get_handler('charm.store') |
617 | 37 | job = StoreIngestJob() | 45 | store = CharmStore() |
618 | 38 | job.setup() | 46 | self.falsify_session(store) |
616 | 39 | self.falsify_session(job) | ||
619 | 40 | ignore, charm = factory.makeCharm(self.db, promulgated=True) | 47 | ignore, charm = factory.makeCharm(self.db, promulgated=True) |
621 | 41 | with patch.object(job, '_store_get', | 48 | with patch.object(store, 'get_charm_info', |
622 | 42 | lambda x: self._mock_data(error=True)): | 49 | lambda x: self._mock_data(error=True)): |
624 | 43 | job.run(charm) | 50 | update_from_store(charm, store, self.log) |
625 | 44 | 51 | ||
626 | 45 | first_address = "cs:%s/%s" % (charm["series"], charm["name"]) | 52 | first_address = "cs:%s/%s" % (charm["series"], charm["name"]) |
627 | 46 | second_address = "cs:~%s/%s/%s" % ( | 53 | second_address = "cs:~%s/%s/%s" % ( |
628 | @@ -54,13 +61,12 @@ | |||
629 | 54 | 61 | ||
630 | 55 | def test_error_handling_warning(self): | 62 | def test_error_handling_warning(self): |
631 | 56 | handler = self.get_handler('charm.store') | 63 | handler = self.get_handler('charm.store') |
635 | 57 | job = StoreIngestJob() | 64 | store = CharmStore() |
636 | 58 | job.setup() | 65 | self.falsify_session(store) |
634 | 59 | self.falsify_session(job) | ||
637 | 60 | ignore, charm = factory.makeCharm(self.db, promulgated=True) | 66 | ignore, charm = factory.makeCharm(self.db, promulgated=True) |
639 | 61 | with patch.object(job, '_store_get', | 67 | with patch.object(store, 'get_charm_info', |
640 | 62 | lambda x: self._mock_data(warning=True)): | 68 | lambda x: self._mock_data(warning=True)): |
642 | 63 | job.run(charm) | 69 | update_from_store(charm, store, self.log) |
643 | 64 | 70 | ||
644 | 65 | first_address = "cs:%s/%s" % (charm["series"], charm["name"]) | 71 | first_address = "cs:%s/%s" % (charm["series"], charm["name"]) |
645 | 66 | second_address = "cs:~%s/%s/%s" % ( | 72 | second_address = "cs:~%s/%s/%s" % ( |
646 | @@ -73,13 +79,13 @@ | |||
647 | 73 | self.assertIn(err_msg, log_messages) | 79 | self.assertIn(err_msg, log_messages) |
648 | 74 | 80 | ||
649 | 75 | def test_run(self): | 81 | def test_run(self): |
653 | 76 | job = StoreIngestJob() | 82 | store = CharmStore() |
654 | 77 | job.setup() | 83 | self.falsify_session(store, '[["2010-12-24", 4], ["2010-12-25", 1]]') |
652 | 78 | self.falsify_session(job, '[["2010-12-24", 4], ["2010-12-25", 1]]') | ||
655 | 79 | ignore, charm = factory.makeCharm(self.db, promulgated=True) | 84 | ignore, charm = factory.makeCharm(self.db, promulgated=True) |
656 | 80 | del charm['store_url'] | 85 | del charm['store_url'] |
659 | 81 | with patch.object(job, '_store_get', lambda x: self._mock_data()): | 86 | with patch.object(store, 'get_charm_info', |
660 | 82 | job.run(charm) | 87 | lambda x: self._mock_data()): |
661 | 88 | update_from_store(charm, store, self.log) | ||
662 | 83 | data = charm['store_data'] | 89 | data = charm['store_data'] |
663 | 84 | self.assertIn('store_checked', data.keys()) | 90 | self.assertIn('store_checked', data.keys()) |
664 | 85 | self.assertEqual(1, data['revision']) | 91 | self.assertEqual(1, data['revision']) |
Thank you.