adt-cloud-service

Merge lp:~fginther/adt-cloud-service/fix-tarmac-return-code into lp:adt-cloud-service

  1. fix-tarmac-return-code
  2. Merge into trunk
Proposed by Francis Ginther
Status: Merged
Approved by: Francis Ginther
Approved revision: 18
Merged at revision: 16
Proposed branch: lp:~fginther/adt-cloud-service/fix-tarmac-return-code
Merge into: lp:adt-cloud-service
Diff against target: 108 lines (+37/-31)
2 files modified
adt_cloud_service/tests/test_functional.py (+1/-1)
called-by-tarmac.py (+36/-30)
To merge this branch: bzr merge lp:~fginther/adt-cloud-service/fix-tarmac-return-code
Reviewer Review Type Date Requested Status
Thomi Richards (community) Approve
Celso Providelo (community) Approve
Francis Ginther Needs Resubmitting
Review via email: mp+253204@code.launchpad.net

Commit message

Fix incorrect return during called-by-tarmac.py when a sub-command fails, make sure it actually exists with a return code. Includes pep8 cleanup of called-by-tarmac.py.

Description of the change

Fix incorrect return during called-by-tarmac.py when a sub-command fails, make sure it actually exists with a return code. Includes pep8 cleanup of called-by-tarmac.py.

To post a comment you must log in.
Revision history for this message
Francis Ginther (fginther) wrote :

There is currently a broken test, so merging this should fail. But I would like to top approve this and make sure it actually rejects the MP this time.

Revision history for this message
Celso Providelo (cprov) wrote :

Francis,

Nice catch, I didn't pay attention when I first reviewed it. _run_cmd() should return value is a little counter intuitive, why doesn't it return 0 (zero) on success and e.returncode on failures ?

review: Needs Information
Revision history for this message
Francis Ginther (fginther) wrote :

Celso, fixed _run_command() as suggested.

review: Needs Resubmitting
Revision history for this message
Celso Providelo (cprov) :
review: Approve
Revision history for this message
Ubuntu CI Bot (uci-bot) wrote :
Download full text (13.2 KiB)

The attempt to merge lp:~fginther/adt-cloud-service/fix-tarmac-return-code into lp:adt-cloud-service failed. Below is the output from the failed tests.

Using base prefix '/usr'
New python executable in /tmp/venv-adt-cloud-service46ow4_fl/bin/python3
Also creating executable in /tmp/venv-adt-cloud-service46ow4_fl/bin/python
Installing setuptools, pip...done.
Running virtualenv with interpreter /usr/bin/python3
Ignoring indexes: https://pypi.python.org/simple/
Downloading/unpacking flask==0.10.1 (from -r requirements.txt (line 1))
  Running setup.py (path:/tmp/venv-adt-cloud-service46ow4_fl/build/flask/setup.py) egg_info for package flask

    warning: no files found matching '*' under directory 'tests'
    warning: no previously-included files matching '*.pyc' found under directory 'docs'
    warning: no previously-included files matching '*.pyo' found under directory 'docs'
    warning: no previously-included files matching '*.pyc' found under directory 'tests'
    warning: no previously-included files matching '*.pyo' found under directory 'tests'
    warning: no previously-included files matching '*.pyc' found under directory 'examples'
    warning: no previously-included files matching '*.pyo' found under directory 'examples'
    no previously-included directories found matching 'docs/_build'
    no previously-included directories found matching 'docs/_themes/.git'
Downloading/unpacking gunicorn==19.3.0 (from -r requirements.txt (line 2))
Downloading/unpacking kombu==3.0.24 (from -r requirements.txt (line 3))
Downloading/unpacking Werkzeug>=0.7 (from flask==0.10.1->-r requirements.txt (line 1))
  Running setup.py (path:/tmp/venv-adt-cloud-service46ow4_fl/build/Werkzeug/setup.py) egg_info for package Werkzeug

    warning: no previously-included files matching '*.pyc' found under directory 'docs'
    warning: no previously-included files matching '*.pyo' found under directory 'docs'
    warning: no previously-included files matching '*.pyc' found under directory 'tests'
    warning: no previously-included files matching '*.pyo' found under directory 'tests'
    warning: no previously-included files matching '*.pyc' found under directory 'examples'
    warning: no previously-included files matching '*.pyo' found under directory 'examples'
    no previously-included directories found matching 'docs/_build'
    no previously-included directories found matching 'docs/_themes'
Downloading/unpacking Jinja2>=2.4 (from flask==0.10.1->-r requirements.txt (line 1))
  Running setup.py (path:/tmp/venv-adt-cloud-service46ow4_fl/build/Jinja2/setup.py) egg_info for package Jinja2

    warning: no files found matching '*' under directory 'custom_fixers'
    warning: no previously-included files matching '*' found under directory 'docs/_build'
    warning: no previously-included files matching '*.pyc' found under directory 'jinja2'
    warning: no previously-included files matching '*.pyc' found under directory 'docs'
    warning: no previously-included files matching '*.pyo' found under directory 'jinja2'
    warning: no previously-included files matching '*.pyo' found under directory 'docs'
Downloading/unpacking itsdangerous>=0.21 (from flask...

lp:~fginther/adt-cloud-service/fix-tarmac-return-code updated
18. By Francis Ginther

Fix broken unit test by adding the version segment.

Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote :

lgtm

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'adt_cloud_service/tests/test_functional.py'
2--- adt_cloud_service/tests/test_functional.py 2015-03-16 03:36:37 +0000
3+++ adt_cloud_service/tests/test_functional.py 2015-03-17 14:53:06 +0000
4@@ -209,7 +209,7 @@
5
6 # make sure the message got on the channel:
7 conn = kombu.Connection("memory://")
8- consumer_queue = conn.SimpleQueue("adt.requests.amd64.nova")
9+ consumer_queue = conn.SimpleQueue("adt.requests.amd64.nova.v1")
10 message = consumer_queue.get(timeout=10.0)
11 message.ack()
12
13
14=== modified file 'called-by-tarmac.py'
15--- called-by-tarmac.py 2015-03-16 14:38:26 +0000
16+++ called-by-tarmac.py 2015-03-17 14:53:06 +0000
17@@ -41,8 +41,7 @@
18
19
20 def _run_command(cmd):
21- '''Run the command and return True. On failure, print error and return
22- False.'''
23+ '''Run the command and return the return code or 0 on success.'''
24
25 print('INFO: Executing: {}'.format(' '.join(cmd)))
26 try:
27@@ -50,8 +49,8 @@
28 except subprocess.CalledProcessError as e:
29 print('ERROR: Calling `{}` failed (with code {})'.format(
30 e.cmd, e.returncode))
31- return False
32- return True
33+ return e.returncode
34+ return 0
35
36
37 def main():
38@@ -59,37 +58,44 @@
39 tempfile.TemporaryDirectory(prefix=VENV_DIR) as ve_dir:
40 pip_cache = os.path.join(pip_dir, 'cache')
41 all_cmds = (
42- ['bzr',
43- 'branch',
44- PIP_CACHE_BRANCH,
45- pip_cache,
46- ],
47- ['virtualenv',
48- '-p',
49- 'python3',
50- ve_dir,
51- ],
52- [os.path.join(ve_dir, 'bin', 'pip'),
53- 'install',
54- '--no-index',
55- '--find-links={}'.format(pip_cache),
56- '-r',
57- 'requirements.txt',
58- ],
59- [os.path.join(ve_dir, 'bin', 'pip'),
60- 'install',
61- '-r',
62- 'test_requirements.txt',
63- ],
64- [os.path.join(ve_dir, 'bin', 'python3'),
65- 'setup.py',
66- 'test',
67+ [
68+ 'bzr',
69+ 'branch',
70+ PIP_CACHE_BRANCH,
71+ pip_cache,
72+ ],
73+ [
74+ 'virtualenv',
75+ '-p',
76+ 'python3',
77+ ve_dir,
78+ ],
79+ [
80+ os.path.join(ve_dir, 'bin', 'pip'),
81+ 'install',
82+ '--no-index',
83+ '--find-links={}'.format(pip_cache),
84+ '-r',
85+ 'requirements.txt',
86+ ],
87+ [
88+ os.path.join(ve_dir, 'bin', 'pip'),
89+ 'install',
90+ '-r',
91+ 'test_requirements.txt',
92+ ],
93+ [
94+ os.path.join(ve_dir, 'bin', 'python3'),
95+ 'setup.py',
96+ 'test',
97 ],
98 )
99 for cmd in all_cmds:
100 ret = _run_command(cmd)
101- if not ret:
102+ if ret:
103+ # A cmd failed, bubble up the return code to the caller
104 return ret
105+ return 0
106
107
108 if __name__ == '__main__':

Subscribers

People subscribed via source and target branches