Merge lp://qastaging/~jml/tarmac/handle-private-branch into lp://qastaging/~ubuntuone-hackers/tarmac/trunk

Proposed by Jonathan Lange
Status: Merged
Approved by: Sidnei da Silva
Approved revision: 433
Merge reported by: Sidnei da Silva
Merged at revision: not available
Proposed branch: lp://qastaging/~jml/tarmac/handle-private-branch
Merge into: lp://qastaging/~ubuntuone-hackers/tarmac/trunk
Diff against target: 173 lines (+75/-27)
2 files modified
tarmac/bin/commands.py (+58/-26)
tarmac/tests/test_commands.py (+17/-1)
To merge this branch: bzr merge lp://qastaging/~jml/tarmac/handle-private-branch
Reviewer Review Type Date Requested Status
Natalia Bidart (community) Approve
Review via email: mp+141910@code.qastaging.launchpad.net

Description of the change

We're getting this error from tarmac on our jenkins server.

10:21:03 E: Not a valid branch: lp:REDACTED
10:21:03 E: An error occurred trying to merge lp:donations-service: 'NoneType' object is not iterable
10:21:03 E: Traceback (most recent call last):
10:21:03 E: File "./bin/tarmac", line 8, in <module>
10:21:03 E: sys.exit(main())
10:21:03 E: File "/mnt/tarmac/tarmac/tarmac/bin/__init__.py", line 30, in main
10:21:03 E: return registry.run(args)
10:21:03 E: File "/mnt/tarmac/tarmac/tarmac/bin/registry.py", line 60, in run
10:21:03 E: return self._run(args)
10:21:03 E: File "/mnt/tarmac/tarmac/tarmac/bin/registry.py", line 48, in _run
10:21:03 E: return run_bzr(args)
10:21:03 E: File "/usr/lib/python2.7/dist-packages/bzrlib/commands.py", line 1131, in run_bzr
10:21:03 E: ret = run(*run_argv)
10:21:03 E: File "/usr/lib/python2.7/dist-packages/bzrlib/commands.py", line 673, in run_argv_aliases
10:21:03 E: return self.run(**all_cmd_args)
10:21:03 E: File "/usr/lib/python2.7/dist-packages/bzrlib/commands.py", line 695, in run
10:21:03 E: return self._operation.run_simple(*args, **kwargs)
10:21:03 E: File "/usr/lib/python2.7/dist-packages/bzrlib/cleanup.py", line 136, in run_simple
10:21:03 E: self.cleanups, self.func, *args, **kwargs)
10:21:03 E: File "/usr/lib/python2.7/dist-packages/bzrlib/cleanup.py", line 166, in _do_with_cleanups
10:21:03 E: result = func(*args, **kwargs)
10:21:03 E: File "/mnt/tarmac/tarmac/tarmac/bin/commands.py", line 379, in run
10:21:03 E: statuses.extend(self._do_merges(branch, verify_command=verify_command))
10:21:03 E: TypeError: 'NoneType' object is not iterable
10:21:03 I: Finished with exitcode 1
Build step 'Execute shell' marked build as failure

This reveals three problems:
 1. "Not a valid branch: lp:REDACTED" is only lightly informative. It doesn't help explain why lp:REDACTED is invalid.
 2. Tarmac crashes when it can't find a branch
 3. Our tarmac bot doesn't have access to lp:REDACTED

The third is a configuration issue that we are addressing. The first and second are bugs in tarmac, which this merge proposal fixes.

It addresses the problem by refactoring out some logic from the merge & check commands for loading branches & merge proposals from Launchpad.

It changes the "Not a valid branch: lp:foo" message to "tarmac-bot could not find lp:foo branch on Launchpad".

To post a comment you must log in.
Revision history for this message
Natalia Bidart (nataliabidart) wrote :

(10:41:54 AM) nessita: jml: looks good. No tests in tarmac? :-/
(10:42:47 AM) jml: nessita: tarmac has tests, yes.
(10:43:32 AM) nessita: jml: no test need modification/adding after these changes?
(10:43:32 AM) jml: launchpadlib is a colossal pain in the neck, which discourages me from testing.
(10:45:48 AM) nessita: I can empathize with that, though... perhaps a test to confirm that the log message is the expected can be added?
(10:45:49 AM) jml: nessita: well, the tests still pass :)
(10:45:57 AM) jml: nessita: yeah, I'll try to add something.
(10:46:02 AM) nessita: jml: thanks

432. By Jonathan Lange

I hate these tests so much.

433. By Jonathan Lange

Test branch not existing.

Revision history for this message
Natalia Bidart (nataliabidart) wrote :

Thanks!

review: Approve

Preview Diff

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

Subscribers

People subscribed via source and target branches