From 4223409141d6cb4f4757d5e3d572bcd6f41bb900 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Thu, 6 Apr 2017 15:03:14 +0100
Subject: [PATCH 01/22] Bump version in docs

---
 docs/conf.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/conf.py b/docs/conf.py
index 6cc3439..dc298cc 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -64,7 +64,7 @@
 # The short X.Y version.
 version = '0.4'
 # The full version, including alpha/beta/rc tags.
-release = '0.4.0'
+release = '0.4.1'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.

From 8df20f45c8b1dfc1dcdd662ea790009adf810871 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Thu, 6 Apr 2017 15:22:13 +0100
Subject: [PATCH 02/22] Add py34 virtualenv to Vagrant, add install_requires to
 tests_require

---
 Vagrantfile | 10 ++++++----
 setup.py    |  3 ++-
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/Vagrantfile b/Vagrantfile
index 06db4bd..3cc66d8 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -61,15 +61,17 @@ Vagrant.configure(2) do |config|
     apt-get -f install -y
 
     # Setup a virtualenv; avoids conflicts, particularly with python-six
-    virtualenv /home/vagrant/virtualenv/orlo
-    source /home/vagrant/virtualenv/orlo/bin/activate
-    echo "source ~/virtualenv/orlo/bin/activate" >> /home/vagrant/.profile
+    virtualenv /home/vagrant/virtualenv/orlo_py27 --python=python2.7
+    virtualenv /home/vagrant/virtualenv/orlo_py34 --python=python3.4
+
+    source /home/vagrant/virtualenv/orlo_py34/bin/activate
+    echo "source ~/virtualenv/orlo_py34/bin/activate" >> /home/vagrant/.profile
 
     pip install --upgrade pip setuptools
 
     cd /vagrant/orlo
     pip install .[test]
-    pip install -r /vagrant/orlo/docs/requirements.txt
+    pip install .[doc]
     python setup.py develop
 
     mkdir -p /etc/orlo /var/log/orlo
diff --git a/setup.py b/setup.py
index d455d75..d86ec97 100755
--- a/setup.py
+++ b/setup.py
@@ -55,7 +55,7 @@
     'mockldap',
     'pytest',
     'tox',
-]
+] + install_requires
 
 
 setup(
@@ -73,6 +73,7 @@
     include_package_data=True,
     install_requires=rtd_requires if on_rtd else install_requires,
     extras_require={
+        'install': install_requires,
         'test': tests_require,
         'doc': rtd_requires,
     },

From 9e62eb0faa6cf9788659725393e6a5cc1e551104 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Tue, 11 Apr 2017 14:47:05 +0100
Subject: [PATCH 03/22] Fix preinst script exiting when orlo user doesn't exist

---
 debian/preinst | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/debian/preinst b/debian/preinst
index c9d2eff..c99dab5 100644
--- a/debian/preinst
+++ b/debian/preinst
@@ -7,9 +7,9 @@ set -e
 case "$1" in
     # If version 0.3.x we need to purge the user and group, as they were 
     # non-system beforehand, and the new postinst creates them with --system
-    upgrade|install)
-        old_uid=$(id -u orlo)
-        old_gid=$(getent group orlo | cut -d: -f3)
+    upgrade)
+        old_uid=$(id -u orlo || true)
+        old_gid=$(getent group orlo || true | cut -d: -f3)
 
         if [ 0$old_uid -gt 1000 ]; then
             deluser orlo || true
@@ -18,4 +18,6 @@ case "$1" in
             delgroup orlo || true
         fi
     ;;
+    install)
+    ;;
 esac

From cb520686a72784b86f2bbd922d130fa66fe6d788 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Tue, 11 Apr 2017 15:02:58 +0100
Subject: [PATCH 04/22] Update debian changelog and setup.py version

---
 debian/changelog | 8 ++++++++
 setup.py         | 2 +-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/debian/changelog b/debian/changelog
index 465c74d..6e1b193 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,11 @@
+orlo (0.4.2) jessie; urgency=medium
+
+  Build changes only
+
+  * Fix preinst script on install
+
+ -- Alex Forbes <github@al4.co.nz>  Tue, 11 Apr 2017 13:52:57 +0000
+
 orlo (0.4.1) jessie; urgency=medium
 
   Build changes only
diff --git a/setup.py b/setup.py
index d86ec97..0595574 100755
--- a/setup.py
+++ b/setup.py
@@ -8,7 +8,7 @@
 
 __location__ = os.path.realpath(os.path.join(os.getcwd(), os.path.dirname(__file__)))
 
-VERSION = '0.4.1'
+VERSION = '0.4.2'
 
 version_file = open(os.path.join(__location__, 'orlo', '_version.py'), 'w')
 version_file.write("__version__ = '{}'".format(VERSION))

From e83022e442dcbec8923845a034c1dd2cb827dab8 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Tue, 11 Apr 2017 16:37:37 +0100
Subject: [PATCH 05/22] Add test for notes

---
 tests/test_route_releases.py | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/tests/test_route_releases.py b/tests/test_route_releases.py
index 6f82675..9296d88 100644
--- a/tests/test_route_releases.py
+++ b/tests/test_route_releases.py
@@ -682,3 +682,21 @@ def test_get_release_with_bad_status(self):
 
         result = self._get_releases(filters=['status=garbage_boz'], expected_status=400)
         self.assertIn('message', result)
+
+    def test_get_release_with_notes(self):
+        """
+        Tests get /release/<id> returns notes
+        """
+        release_id = self._create_release()
+        response = self._post_releases_notes(release_id, "this is a test message")
+        self.assertEqual(204, response.status_code)
+
+        output = self._get_releases(release_id=release_id)
+        self.assertIn('notes', output['releases'][0])
+
+        notes = output['releases'][0]['notes']
+        self.assertIn(
+            'test note lorem ipsum',
+            notes
+        )
+

From 96a5182c00753235e7654cdc7b26384347910a63 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Tue, 11 Apr 2017 16:38:09 +0100
Subject: [PATCH 06/22] Fix release notes relationship

---
 orlo/orm.py | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/orlo/orm.py b/orlo/orm.py
index 210fe70..2a956a4 100644
--- a/orlo/orm.py
+++ b/orlo/orm.py
@@ -65,6 +65,7 @@ class Release(db.Model):
     user = db.Column(db.String, nullable=False)
     team = db.Column(db.String)
     packages = db.relationship("Package", backref=db.backref("release"))
+    notes = db.relationship("ReleaseNote", backref=db.backref("release"))
 
     def __init__(self, platforms, user, team=None, references=None):
         # platforms and references are stored as strings in DB but really are lists
@@ -101,6 +102,7 @@ def to_dict(self):
             'metadata': metadata,
             'user': self.user,
             'team': self.team,
+            'notes': [n.content for n in self.notes],
         }
 
     def start(self):
@@ -215,9 +217,7 @@ class ReleaseNote(db.Model):
 
     id = db.Column(UUIDType, primary_key=True, unique=True)
     content = db.Column(db.Text, nullable=False)
-
     release_id = db.Column(UUIDType, db.ForeignKey("release.id"))
-    release = db.relationship("Release", backref=db.backref('notes', order_by=id))
 
     def __init__(self, release_id, content):
         self.id = uuid.uuid4()

From f680c969a15d3f32a81c1bf90a3b77d6812db8a0 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Tue, 11 Apr 2017 16:38:34 +0100
Subject: [PATCH 07/22] Set log permissions in vagrant

---
 Vagrantfile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Vagrantfile b/Vagrantfile
index 3cc66d8..7c6d004 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -75,6 +75,7 @@ Vagrant.configure(2) do |config|
     python setup.py develop
 
     mkdir -p /etc/orlo /var/log/orlo
+    chown -R vagrant:vagrant /var/log/orlo
     # echo -e "[db]\nuri=postgres://orlo:password@192.168.57.100" > /etc/orlo/orlo.ini
 
 

From 5c606b2a5628ead8724344252913668c82db9c22 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Tue, 11 Apr 2017 17:12:40 +0100
Subject: [PATCH 08/22] Add alembic migration: "add unique constraints"

---
 .../0868747e62ff_add_unique_constraints.py    | 33 +++++++++++++++++++
 1 file changed, 33 insertions(+)
 create mode 100644 orlo/migrations/0868747e62ff_add_unique_constraints.py

diff --git a/orlo/migrations/0868747e62ff_add_unique_constraints.py b/orlo/migrations/0868747e62ff_add_unique_constraints.py
new file mode 100644
index 0000000..09dc36e
--- /dev/null
+++ b/orlo/migrations/0868747e62ff_add_unique_constraints.py
@@ -0,0 +1,33 @@
+"""Add unique constraints
+
+Revision ID: 0868747e62ff
+Revises: e60a77e44da8
+Create Date: 2017-04-11 16:10:42.109777
+
+"""
+from alembic import op
+import sqlalchemy as sa
+
+
+# revision identifiers, used by Alembic.
+revision = '0868747e62ff'
+down_revision = 'e60a77e44da8'
+branch_labels = ()
+depends_on = None
+
+def upgrade():
+    op.create_unique_constraint(None, 'package', ['id'])
+    op.create_unique_constraint(None, 'package_result', ['id'])
+    op.create_unique_constraint(None, 'platform', ['id'])
+    op.create_unique_constraint(None, 'release', ['id'])
+    op.create_unique_constraint(None, 'release_metadata', ['id'])
+    op.create_unique_constraint(None, 'release_note', ['id'])
+
+
+def downgrade():
+    op.drop_constraint(None, 'release_note', type_='unique')
+    op.drop_constraint(None, 'release_metadata', type_='unique')
+    op.drop_constraint(None, 'release', type_='unique')
+    op.drop_constraint(None, 'platform', type_='unique')
+    op.drop_constraint(None, 'package_result', type_='unique')
+    op.drop_constraint(None, 'package', type_='unique')

From ccff06db72e2636c189e2a939a50e9878917f8a5 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Fri, 28 Apr 2017 16:00:03 +0100
Subject: [PATCH 09/22] Set default database path to /var/lib/orlo/orlo.db

"sqlite://" is a problematic default because an in-memory database
means having a separate database per-thread.
---
 Vagrantfile  | 2 +-
 etc/orlo.ini | 2 +-
 orlo/app.py  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/Vagrantfile b/Vagrantfile
index 7c6d004..80fd75e 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -86,7 +86,7 @@ Vagrant.configure(2) do |config|
 
   config.vm.define "jessie" do |jessie|
     jessie.vm.box = "bento/debian-8.7"
-    jessie.vm.network "forwarded_port", guest: 5000, host: 5000
+    jessie.vm.network "forwarded_port", guest: 5000, host: 5100
     jessie.vm.network "private_network", ip: "192.168.57.20"
     jessie.vm.provision "shell", inline: <<-SHELL
     SHELL
diff --git a/etc/orlo.ini b/etc/orlo.ini
index 38861a2..ca90ce8 100644
--- a/etc/orlo.ini
+++ b/etc/orlo.ini
@@ -1,5 +1,5 @@
 [db]
-uri = sqlite://
+uri = sqlite:////var/lib/orlo/orlo.db
 ; Mapped to SQLALCHEMY_ECHO
 echo_queries = false
 
diff --git a/orlo/app.py b/orlo/app.py
index 7016007..96d7f8e 100644
--- a/orlo/app.py
+++ b/orlo/app.py
@@ -98,7 +98,7 @@ def load(self):
 
 
 if config.getboolean('security', 'enabled') and \
-                config.get('security', 'secret_key') == 'change_me':
+        config.get('security', 'secret_key') == 'change_me':
     raise OrloStartupError(
         "Security is enabled, please configure security:secret_key in orlo.ini")
 

From 6dc70d853ae00193118321c69da231b8367b0bc1 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Fri, 28 Apr 2017 16:23:18 +0100
Subject: [PATCH 10/22] Return 404 status code when no releases are found

---
 orlo/routes/releases.py      |  5 +++++
 tests/test_route_releases.py | 14 ++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/orlo/routes/releases.py b/orlo/routes/releases.py
index 424ccf1..63c405b 100644
--- a/orlo/routes/releases.py
+++ b/orlo/routes/releases.py
@@ -353,5 +353,10 @@ def get_releases(release_id=None):
     # error
     db.session.execute(query)
 
+    if query.count() is 0:
+        response = jsonify(message="No releases found", releases=[])
+        response.status_code = 404
+        return response
+
     return Response(stream_json_list('releases', query),
                     content_type='application/json')
diff --git a/tests/test_route_releases.py b/tests/test_route_releases.py
index 9296d88..4c45f3f 100644
--- a/tests/test_route_releases.py
+++ b/tests/test_route_releases.py
@@ -218,6 +218,20 @@ def test_get_single_release_invalid(self):
         r = self._get_releases(release_id="not_a_valid_uuid",
                                expected_status=400)
 
+    def test_get_single_release_404(self):
+        """
+        Test that we return 404 with a valid but non-existent release ID
+        """
+        r = self._get_releases(release_id=str(uuid.uuid4()),
+                               expected_status=404)
+
+    def test_get_filtered_releases_404(self):
+        """
+        Test that we return 404 with a filter that returns no results
+        """
+        results = self._get_releases(expected_status=404,
+            filters=['package_name=non-existent-package'])
+
     def test_get_releases(self):
         """
         Test the list of releases

From 1d77f23737f58376d155c3c5127f9286957456e1 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Fri, 28 Apr 2017 16:30:12 +0100
Subject: [PATCH 11/22] Make /packages behaviour consistent with recent
 /releases changes

* Don't raise InvalidUsage without filters
* Set default limit to 100
* Return 404 when no packages are found
---
 orlo/routes/packages.py      | 13 ++++++++-----
 tests/test_route_packages.py | 15 ++++++++++++---
 2 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/orlo/routes/packages.py b/orlo/routes/packages.py
index 8af718d..b26c432 100644
--- a/orlo/routes/packages.py
+++ b/orlo/routes/packages.py
@@ -37,14 +37,12 @@ def get_packages(package_id=None):
         if not is_uuid(package_id):
             raise InvalidUsage("Package ID given is not a valid UUID")
         query = queries.get_package(package_id)
-    elif len([x for x in request.args.keys()]) == 0:
-        raise InvalidUsage("Please specify a filter. See "
-                           "http://orlo.readthedocs.org/en/latest/rest.html"
-                           "#get--packages for more info")
     else:  # Bit more complex
         # Flatten args, as the ImmutableDict puts some values in a list when
         # expanded
-        args = {}
+        args = {
+            'limit': 100
+        }
         for k in request.args.keys():
             if k in booleans:
                 args[k] = str_to_bool(request.args.get(k))
@@ -56,5 +54,10 @@ def get_packages(package_id=None):
     # error
     db.session.execute(query)
 
+    if query.count() is 0:
+        response = jsonify(message="No packages found", packages=[])
+        response.status_code = 404
+        return response
+
     return Response(stream_json_list('packages', query),
                     content_type='application/json')
diff --git a/tests/test_route_packages.py b/tests/test_route_packages.py
index 415cfbb..76e8c4b 100644
--- a/tests/test_route_packages.py
+++ b/tests/test_route_packages.py
@@ -1,6 +1,7 @@
 from __future__ import print_function
 from test_route_base import OrloHttpTest
 import json
+import uuid
 
 __author__ = 'alforbes'
 
@@ -34,14 +35,22 @@ def _get_packages(self, package_id=None, filters=None, expected_status=200):
         r_json = json.loads(results_response.data.decode('utf-8'))
         return r_json
 
-    def test_packages(self):
+    def test_packages_invalid_uuid_returns_400(self):
         """
-        Test that we get 400 and a message with no params
+        Test that we get 400 with an invald uuid
         """
-        response = self.client.get('/packages')
+        response = self.client.get('/packages/not-a-valid-uuid')
         self.assert400(response)
         self.assertIn('message', response.json)
 
+    def test_packages_missing_package_returns_404(self):
+        """
+        Test that we get 404 when there are no packages
+        """
+        response = self.client.get('/packages/{}'.format(uuid.uuid4()))
+        self.assert404(response)
+        self.assertIn('message', response.json)
+
     def test_get_single_package(self):
         """
         Fetch a single release

From 98b6f4b47a41cf8785625c23516aa0c877c30e73 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Wed, 10 May 2017 18:48:57 +0100
Subject: [PATCH 12/22] Add [client] section to config

orloclient will read this to find the orlo server
---
 etc/orlo.ini | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/etc/orlo.ini b/etc/orlo.ini
index ca90ce8..4fd6807 100644
--- a/etc/orlo.ini
+++ b/etc/orlo.ini
@@ -31,3 +31,7 @@ user_base_dn = ou=people,ou=example,o=test
 [logging]
 directory = /var/log/orlo
 level = info
+
+; orloclient will read /etc/orlo/orlo.ini to set the default uri
+[client]
+uri = http://localhost:8080

From 538e3f15beed9fb63c17d007453f36903e595d5b Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Wed, 17 May 2017 13:53:15 +0100
Subject: [PATCH 13/22] Fix tests from earlier status code change

Changed the urls to return 404 if no results were found, this fixes the
tests to account for that.
---
 orlo/routes/releases.py      |  3 +--
 tests/test_queries.py        |  7 ++++---
 tests/test_route_releases.py | 34 ++++++++++++++++++++++------------
 3 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/orlo/routes/releases.py b/orlo/routes/releases.py
index 63c405b..3f9d4db 100644
--- a/orlo/routes/releases.py
+++ b/orlo/routes/releases.py
@@ -355,8 +355,7 @@ def get_releases(release_id=None):
 
     if query.count() is 0:
         response = jsonify(message="No releases found", releases=[])
-        response.status_code = 404
-        return response
+        return response, 404
 
     return Response(stream_json_list('releases', query),
                     content_type='application/json')
diff --git a/tests/test_queries.py b/tests/test_queries.py
index 44fa0e5..0eda298 100644
--- a/tests/test_queries.py
+++ b/tests/test_queries.py
@@ -265,9 +265,10 @@ def test_package_versions(self):
         """
         Test package_versions
 
-        In this test, we create two releases. packageOne succeeds in both but packageTwo fails
-        in the second, therefore the current version for packageOne should be the second release,
-        but packageTwo should remain with the first version
+        In this test, we create two releases. packageOne succeeds in both but 
+        packageTwo fails in the second, therefore the current version for 
+        packageOne should be the second release, but packageTwo should remain 
+        with the first version
         """
         rid1 = self._create_release(platforms=['platformOne'])
         pid1 = self._create_package(rid1, name='packageOne', version='1.0.1')
diff --git a/tests/test_route_releases.py b/tests/test_route_releases.py
index 4c45f3f..72e6d5e 100644
--- a/tests/test_route_releases.py
+++ b/tests/test_route_releases.py
@@ -180,6 +180,10 @@ class TestGetContract(OrloHttpTest):
     def _get_releases(self, release_id=None, filters=None, expected_status=200):
         """
         Perform a GET to /releases with optional filters
+        
+        :param string release_id: release uuid
+        :param list filters: list of filters
+        :param expected_status: status code to expect (or None to not assert)
         """
 
         if release_id:
@@ -193,11 +197,12 @@ def _get_releases(self, release_id=None, filters=None, expected_status=200):
             path, content_type='application/json',
         )
 
-        try:
-            self.assertEqual(results_response.status_code, expected_status)
-        except AssertionError as err:
-            print(results_response.data)
-            raise
+        if expected_status:
+            try:
+                self.assertEqual(results_response.status_code, expected_status)
+            except AssertionError as err:
+                print(results_response.data)
+                raise
         r_json = json.loads(results_response.data.decode('utf-8'))
         return r_json
 
@@ -326,10 +331,10 @@ def _get_releases_time_filter(self, field, finished=False):
         tomorrow = (now + timedelta(days=1)).strftime(t_format)
 
         r_yesterday = self._get_releases(
-            filters=['{}={}'.format(field, yesterday)]
+            filters=['{}={}'.format(field, yesterday)], expected_status=None
         )
         r_tomorrow = self._get_releases(
-            filters=['{}={}'.format(field, tomorrow)]
+            filters=['{}={}'.format(field, tomorrow)], expected_status=None
         )
 
         return r_yesterday, r_tomorrow
@@ -348,7 +353,8 @@ def test_get_release_filter_ftime_null(self):
         Filter on releases that don't have a finish time
         """
         releases = self._get_releases(
-            filters=['{}={}'.format('ftime', 'null')]
+            filters=['{}={}'.format('ftime', 'null')],
+            expected_status=404,
         )
         self.assertEqual(len(releases['releases']), 0)
 
@@ -394,7 +400,8 @@ def test_get_release_filter_duration_lt(self):
         r = self._get_releases(filters=['duration_lt=10'])
         self.assertEqual(3, len(r['releases']))
 
-        r = self._get_releases(filters=['duration_lt=0'])
+        r = self._get_releases(filters=['duration_lt=0'],
+                               expected_status=404)
         self.assertEqual(0, len(r['releases']))
 
     def test_get_release_filter_duration_gt(self):
@@ -404,7 +411,8 @@ def test_get_release_filter_duration_gt(self):
         for _ in range(0, 3):
             self._create_finished_release()
 
-        r = self._get_releases(filters=['duration_gt=10'])
+        r = self._get_releases(filters=['duration_gt=10'],
+                               expected_status=404)
         self.assertEqual(0, len(r['releases']))
 
         r = self._get_releases(filters=['duration_gt=0'])
@@ -468,7 +476,8 @@ def test_get_release_filter_reference(self):
             self._create_package(rid)
 
         first_results = self._get_releases(filters=['reference=REF'])
-        second_results = self._get_releases(filters=['reference=ZERO'])
+        second_results = self._get_releases(filters=['reference=ZERO'],
+                                            expected_status=404)
 
         self.assertEqual(len(first_results['releases']), 3)
         self.assertEqual(len(second_results['releases']), 0)
@@ -642,7 +651,8 @@ def test_get_release_filter_rollback_and_status(self):
                                                      'package_status=NOT_STARTED'])
         # should be zero
         third_results = self._get_releases(filters=['package_rollback=False',
-                                                    'package_status=SUCCESSFUL'])
+                                                    'package_status=SUCCESSFUL'],
+                                           expected_status=404)
 
         self.assertEqual(len(first_results['releases']), 3)
         self.assertEqual(len(second_results['releases']), 2)

From 7e1c2981ca24646e8e0d5b0cccf6cb7ed112561e Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Wed, 17 May 2017 13:54:12 +0100
Subject: [PATCH 14/22] Merge the package and release queries into one
 build_query function

Requires a few if statements, but generally less repetition
---
 orlo/queries.py              | 64 ++++++++++++++++--------------------
 orlo/routes/packages.py      |  2 +-
 orlo/routes/releases.py      |  2 +-
 tests/test_queries.py        | 61 ++++++++++++++++++----------------
 tests/test_route_packages.py |  2 +-
 5 files changed, 64 insertions(+), 67 deletions(-)

diff --git a/orlo/queries.py b/orlo/queries.py
index 77bf923..9c4789c 100644
--- a/orlo/queries.py
+++ b/orlo/queries.py
@@ -246,41 +246,57 @@ def apply_package_filters(query, args):
     return query
 
 
-def releases(limit=None, offset=None, asc=None, **kwargs):
+def build_query(object_type, limit=None, offset=None, asc=None, **kwargs):
     """
     Return whole releases, based on filters
 
+    :param object_type: Object type to query, Release or Package
     :param limit: Max number of results to return
     :param offset: Offset results. Provides pagination when combined with limit.
     :param asc: Sort ascending instead of the default descending order
     :param kwargs: Request arguments
     :return:
     """
+    if object_type is Release:
+        filter_function = apply_filters
+    elif object_type is Package:
+        filter_function = apply_package_filters
+    else:
+        raise OrloError("build_query does not support object {}".format(
+            object_type
+        ))
 
     if any(field.startswith('package_') for field in kwargs.keys()) \
-            or "status" in kwargs.keys():
-        # Package attributes need the join, as does status as it's really a
-        # package attribute
-        query = db.session.query(Release).join(Package)
+            or ("status" in kwargs.keys() and object_type is Release):
+        if object_type is not Release:
+            raise InvalidUsage(
+                "'package_' parameters are only valid for Release queries. "
+                "(hint: retry without the package_ prefix)")
+        else:
+            # Package attributes need the join, as does status as it's really a
+            # package attribute
+            query = db.session.query(object_type).join(Package)
     else:
         # No need to join on package if none of our params need it
-        query = db.session.query(Release)
+        query = db.session.query(object_type)
 
     for key in kwargs.keys():
         if isinstance(kwargs[key], bool):
             continue
         if kwargs[key].lower() in ['null', 'none']:
             kwargs[key] = None
+
     try:
-        query = apply_filters(query, kwargs)
+        query = filter_function(query, kwargs)
     except AttributeError as e:
         raise InvalidUsage(
-            "An invalid field was specified: {}".format(e.args[0]))
+            "An invalid field for table {} was specified: {}".format(
+                object_type.__tablename__, e.args[0]))
 
     if asc:
-        stime_field = Release.stime.asc
+        stime_field = object_type.stime.asc
     else:
-        stime_field = Release.stime.desc
+        stime_field = object_type.stime.desc
 
     query = query.order_by(stime_field())
 
@@ -299,32 +315,8 @@ def releases(limit=None, offset=None, asc=None, **kwargs):
 
     return query
 
-
-def packages(**kwargs):
-    """
-    Return a list of packages
-
-    Fairly simple at present, as this is not envisioned to be frequently used.
-
-    :param kwargs: Filters
-    :return:
-    """
-    query = db.session.query(Package)
-
-    for key in kwargs.keys():
-        if isinstance(kwargs[key], bool):
-            continue
-        if kwargs[key].lower() in ['null', 'none']:
-            kwargs[key] = None
-    try:
-        query = apply_package_filters(query, kwargs)
-    except AttributeError as e:
-        raise InvalidUsage("An invalid field was specified")
-
-    query = query.order_by(Package.stime.asc())
-
-    return query
-
+def packages():
+    pass
 
 def user_summary(platform=None):
     """
diff --git a/orlo/routes/packages.py b/orlo/routes/packages.py
index b26c432..1c17e55 100644
--- a/orlo/routes/packages.py
+++ b/orlo/routes/packages.py
@@ -48,7 +48,7 @@ def get_packages(package_id=None):
                 args[k] = str_to_bool(request.args.get(k))
             else:
                 args[k] = request.args.get(k)
-        query = queries.packages(**args)
+        query = queries.build_query(Package, **args)
 
     # Execute eagerly to avoid confusing stack traces within the Response on
     # error
diff --git a/orlo/routes/releases.py b/orlo/routes/releases.py
index 3f9d4db..73de69c 100644
--- a/orlo/routes/releases.py
+++ b/orlo/routes/releases.py
@@ -347,7 +347,7 @@ def get_releases(release_id=None):
                 args[k] = str_to_bool(request.args.get(k))
             else:
                 args[k] = request.args.get(k)
-        query = queries.releases(**args)
+        query = queries.build_query(Release, **args)
 
     # Execute eagerly to avoid confusing stack traces within the Response on
     # error
diff --git a/tests/test_queries.py b/tests/test_queries.py
index 0eda298..e9091f7 100644
--- a/tests/test_queries.py
+++ b/tests/test_queries.py
@@ -5,6 +5,7 @@
 import orlo.queries
 import orlo.exceptions
 import orlo.stats
+from orlo.orm import Release, Package
 from time import sleep
 import sqlalchemy.orm
 import uuid
@@ -688,12 +689,29 @@ def test_rollback_false(self):
         self.assertEqual(1, result[0][0])
 
 
-class ReleasesTest(OrloQueryTest):
+class TestBuildQuery(OrloQueryTest):
     """
-    Test the releases method
+    Test the build_query method
     """
 
-    def test_releases_with_bad_limit(self):
+    def test_build_query_returns_query_with_Release(self):
+        """
+        Test that
+        :return:
+        """
+        result = orlo.queries.build_query(Release)
+        self.assertIsInstance(result, sqlalchemy.orm.query.Query)
+
+
+    def test_build_query_returns_query_with_Package(self):
+        """
+        Test that
+        :return:
+        """
+        result = orlo.queries.build_query(Package)
+        self.assertIsInstance(result, sqlalchemy.orm.query.Query)
+
+    def test_bad_query_with_bad_limit(self):
         """
         Test releases raises InvalidUsage when limit is not an int
         """
@@ -701,9 +719,9 @@ def test_releases_with_bad_limit(self):
             'limit': 'bad_limit',
         }
         with self.assertRaises(orlo.exceptions.InvalidUsage):
-            orlo.queries.releases(**args)
+            orlo.queries.build_query(Release, **args)
 
-    def test_releases_with_bad_offset(self):
+    def test_bad_query_with_bad_offset(self):
         """
         Test releases raises InvalidUsage when offset is not an int
         """
@@ -711,7 +729,16 @@ def test_releases_with_bad_offset(self):
             'offset': 'bad_offset',
         }
         with self.assertRaises(orlo.exceptions.InvalidUsage):
-            orlo.queries.releases(**args)
+            orlo.queries.build_query(Release, **args)
+
+    def test_with_package(self):
+        """
+        Test that query returned by get_package works
+        """
+        rid = self._create_release()
+        pid = self._create_package(rid, rollback=False)
+        result = orlo.queries.build_query(Package).all()
+        self.assertEqual(result[0].id, pid)
 
 
 class GetPackageTest(OrloQueryTest):
@@ -736,25 +763,3 @@ def test_get_package_works(self):
 
         self.assertEqual(len(result), 1)
         self.assertEqual(result[0].id, pid)
-
-
-class PackagesTests(OrloQueryTest):
-    """
-    Test the queries.packages method
-    """
-    def test_packages_returns_query(self):
-        """
-        Test that
-        :return:
-        """
-        result = orlo.queries.packages()
-        self.assertIsInstance(result, sqlalchemy.orm.query.Query)
-
-    def test_packages_works(self):
-        """
-        Test that query returned by get_package works
-        """
-        rid = self._create_release()
-        pid = self._create_package(rid, rollback=False)
-        result = orlo.queries.packages().all()
-        self.assertEqual(result[0].id, pid)
diff --git a/tests/test_route_packages.py b/tests/test_route_packages.py
index 76e8c4b..6afdfa0 100644
--- a/tests/test_route_packages.py
+++ b/tests/test_route_packages.py
@@ -13,7 +13,7 @@ class GetPackagesTest(OrloHttpTest):
 
     def _get_packages(self, package_id=None, filters=None, expected_status=200):
         """
-        Perform a GET to /releases with optional filters
+        Perform a GET to /packages with optional filters
         """
 
         if package_id:

From ce75c28bed27906e8e4b54f8f5e856769c6b5c00 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Wed, 17 May 2017 14:33:27 +0100
Subject: [PATCH 15/22] Fix sometimes not returning 404 with zero results

`is` vs `==` ... I was under the impression that low ints were references
to the same object. Bad to be exploiting this in any case.
---
 orlo/routes/packages.py | 5 ++---
 orlo/routes/releases.py | 2 +-
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/orlo/routes/packages.py b/orlo/routes/packages.py
index 1c17e55..4ed8893 100644
--- a/orlo/routes/packages.py
+++ b/orlo/routes/packages.py
@@ -54,10 +54,9 @@ def get_packages(package_id=None):
     # error
     db.session.execute(query)
 
-    if query.count() is 0:
+    if query.count() == 0:
         response = jsonify(message="No packages found", packages=[])
-        response.status_code = 404
-        return response
+        return response, 404
 
     return Response(stream_json_list('packages', query),
                     content_type='application/json')
diff --git a/orlo/routes/releases.py b/orlo/routes/releases.py
index 73de69c..0fe7c38 100644
--- a/orlo/routes/releases.py
+++ b/orlo/routes/releases.py
@@ -353,7 +353,7 @@ def get_releases(release_id=None):
     # error
     db.session.execute(query)
 
-    if query.count() is 0:
+    if query.count() == 0:
         response = jsonify(message="No releases found", releases=[])
         return response, 404
 

From 5432c7c6fc91a7eeb5b3afce31e7a7ab056dd078 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Wed, 17 May 2017 14:34:50 +0100
Subject: [PATCH 16/22] Fix auth test after 404 change

Just use a url that doesn't return 404 if there are no packages
---
 tests/test_auth.py | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/tests/test_auth.py b/tests/test_auth.py
index f62e33d..766a7f9 100644
--- a/tests/test_auth.py
+++ b/tests/test_auth.py
@@ -237,25 +237,22 @@ def test_with_bad_login(self, mock_verify_password):
 
 class TestReleasesAuth(OrloAuthTest):
     """
-    Test auth against a real url, /releases
+    Test auth against real urls
 
     Bit more complicated as we have to do some POST requests
     """
-    URL_PATH = '/releases'
-
     def test_get_returns_200(self):
         """
         No auth required for GETs
         """
-        url = self.URL_PATH + '?platform=foo'
-        response = self.client.get(url)
+        response = self.client.get('/info')
         self.assert200(response)
 
     def test_post_releases_returns_401(self):
         """
         Test auth fails
         """
-        response = self.client.post(self.URL_PATH, data={'foo': 'bar'})
+        response = self.client.post('/releases', data={'foo': 'bar'})
         self.assert401(response)
 
     @patch('orlo.user_auth.verify_password_file')
@@ -266,7 +263,7 @@ def test_post_releases_with_token_returns_400(
         """
         token = self.get_token()
         response = self.post_with_token_auth(
-            self.URL_PATH, token=token, data={'foo': 'bar'},
+            '/releases', token=token, data={'foo': 'bar'},
         )
         self.assert400(response)
         self.assertIn(b'message', response.data)

From f17d0a2460c5a664d0b251204d1c139379bba04a Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Thu, 11 May 2017 16:15:10 +0100
Subject: [PATCH 17/22] Allow excluding packages from versions, if they are
 part of an incomplete release

---
 orlo/queries.py       | 23 +++++++++++++++++++----
 tests/test_queries.py | 42 +++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 60 insertions(+), 5 deletions(-)

diff --git a/orlo/queries.py b/orlo/queries.py
index 9c4789c..b329b76 100644
--- a/orlo/queries.py
+++ b/orlo/queries.py
@@ -44,6 +44,7 @@ def filter_release_status(query, status):
     :param status: The status to filter on
     :return:
     """
+    app.logger.info("Filtering release status on {}".format(status))
     enums = Package.status.property.columns[0].type.enums
     if status not in enums:
         raise InvalidUsage("Invalid package status, {} is not in {}".format(
@@ -60,6 +61,8 @@ def filter_release_status(query, status):
     elif status in ["FAILED", "IN_PROGRESS"]:
         # ANY package can match for this status to apply to the release
         query = query.filter(Release.packages.any(Package.status == status))
+    else:
+        app.logger.debug("Not filtering query on release status")
     return query
 
 
@@ -457,7 +460,7 @@ def package_list(platform=None):
     return query
 
 
-def package_versions(platform=None):
+def package_versions(platform=None, exclude_partial_releases=False):
     """
     List the current version of all packages
 
@@ -466,6 +469,10 @@ def package_versions(platform=None):
     release time
 
     :param platform: Platform to filter on
+    :param exclude_partial_releases: If false, a package, that is part of a 
+        release, which has other packages IN_PROGRESS, will not be considered 
+        the current version. If true, a package will be the current version 
+        as long as its status is SUCCESSFUL. 
     """
 
     # Sub query gets a list of successful packages by last successful release
@@ -474,10 +481,18 @@ def package_versions(platform=None):
             Package.name.label('name'),
             db.func.max(Package.stime).label('max_stime')) \
         .filter(Package.status == 'SUCCESSFUL')
+
+    if platform or exclude_partial_releases:
+        # Need to join on Release
+        sub_q = sub_q.join(Release)
     if platform:  # filter by platform
-        sub_q = sub_q \
-            .join(Release) \
-            .filter(Release.platforms.any(Platform.name == platform))
+        sub_q = sub_q.filter(Release.platforms.any(Platform.name == platform))
+    if exclude_partial_releases:
+        # Filter out packages which are part of a release in progress,
+        # see issue#52
+        app.logger.critical("FOO")
+        sub_q = filter_release_status(sub_q, status="SUCCESSFUL")
+
     sub_q = sub_q \
         .group_by(Package.name) \
         .subquery()
diff --git a/tests/test_queries.py b/tests/test_queries.py
index e9091f7..ce9f182 100644
--- a/tests/test_queries.py
+++ b/tests/test_queries.py
@@ -5,15 +5,21 @@
 import orlo.queries
 import orlo.exceptions
 import orlo.stats
-from orlo.orm import Release, Package
+from orlo.orm import Release, Package, app
 from time import sleep
 import sqlalchemy.orm
 import uuid
 from orlo.util import is_uuid
+import logging
 
 __author__ = 'alforbes'
 
 
+logger = logging.getLogger()
+logger.setLevel(logging.DEBUG)
+
+
+
 """
 Test the query functions in queries.py
 """
@@ -262,6 +268,8 @@ def test_package_summary_with_ftime_negative(self):
 
         self.assertEqual(len(result), 0)
 
+
+class TestPackageVersions(OrloQueryTest):
     def test_package_versions(self):
         """
         Test package_versions
@@ -331,6 +339,38 @@ def test_package_versions_with_rollback(self):
         # Correct versions:
         self.assertIn(('packageOne', '1.0.1'), versions)
 
+    def test_package_versions_excludes_partial_releases(self):
+        """ Test that we exclude releases in progress
+        
+        When partial_releases is False (the default). See issue#52.
+        """
+        # Full release of both components
+        rid1 = self._create_release(platforms=['platformOne'])
+        pid1 = self._create_package(rid1, name='packageOne', version='1.0')
+        pid2 = self._create_package(rid1, name='packageTwo', version='1.0')
+        self._start_package(pid1)
+        self._stop_package(pid1)
+        self._start_package(pid2)
+        self._stop_package(pid2)
+        sleep(0.1)  # To ensure some time separation
+
+        # Partial release
+        rid2 = self._create_release(platforms=['platformOne'])
+        pid1 = self._create_package(rid2, name='packageOne', version='2.0')
+        pid2 = self._create_package(rid2, name='packageTwo', version='2.0')
+        self._start_package(pid1)
+        self._stop_package(pid1)
+        self._start_package(pid2)
+        # note - have not stopped packageTwo
+
+        result = orlo.queries.package_versions(
+            exclude_partial_releases=True, platform='platformOne').all()
+        self.assertEqual(len(result), 2)
+        for pkg, ver in result:
+            # Should both be version 1.0, despite packageOne 2.0 being
+            # successful
+            self.assertEqual(ver, '1.0')
+
 
 class TestInfo(OrloQueryTest):
     """

From d0aa59b81199d8a31f0f8dce513f342e7f7e70b4 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Wed, 17 May 2017 15:53:49 +0100
Subject: [PATCH 18/22] Add by_release to /info/packages/versions url

 * Renamed exclude_partial_releases param to by_release
 * Added 'behaviour' section in config, with corresponding
versions_by_release parameter
---
 orlo/config.py           |  3 +++
 orlo/queries.py          | 16 ++++++++--------
 orlo/routes/info.py      | 18 ++++++++++++++++--
 tests/test_queries.py    |  2 +-
 tests/test_route_info.py |  6 ++++++
 5 files changed, 34 insertions(+), 11 deletions(-)

diff --git a/orlo/config.py b/orlo/config.py
index 2fa950e..afcaf5e 100644
--- a/orlo/config.py
+++ b/orlo/config.py
@@ -61,4 +61,7 @@
 config.set('logging', 'directory', defaults['ORLO_LOGDIR'])  # "disabled" for no
                                                              # log files
 
+config.add_section('behaviour')
+config.set('behaviour', 'versions_by_release', 'false')
+
 config.read(defaults['ORLO_CONFIG'])
diff --git a/orlo/queries.py b/orlo/queries.py
index b329b76..768c720 100644
--- a/orlo/queries.py
+++ b/orlo/queries.py
@@ -460,7 +460,7 @@ def package_list(platform=None):
     return query
 
 
-def package_versions(platform=None, exclude_partial_releases=False):
+def package_versions(platform=None, by_release=False):
     """
     List the current version of all packages
 
@@ -469,10 +469,11 @@ def package_versions(platform=None, exclude_partial_releases=False):
     release time
 
     :param platform: Platform to filter on
-    :param exclude_partial_releases: If false, a package, that is part of a 
-        release, which has other packages IN_PROGRESS, will not be considered 
-        the current version. If true, a package will be the current version 
-        as long as its status is SUCCESSFUL. 
+    :param bool by_release: If true, a package, that is part of a release which
+        is not SUCCESSFUL, will not be considered the current version, even
+        if its own status is SUCCESSFUL. If false, a package will be the
+        current version as long as its own status is SUCCESSFUL.
+        Default: False.
     """
 
     # Sub query gets a list of successful packages by last successful release
@@ -482,15 +483,14 @@ def package_versions(platform=None, exclude_partial_releases=False):
             db.func.max(Package.stime).label('max_stime')) \
         .filter(Package.status == 'SUCCESSFUL')
 
-    if platform or exclude_partial_releases:
+    if platform or by_release:
         # Need to join on Release
         sub_q = sub_q.join(Release)
     if platform:  # filter by platform
         sub_q = sub_q.filter(Release.platforms.any(Platform.name == platform))
-    if exclude_partial_releases:
+    if by_release:
         # Filter out packages which are part of a release in progress,
         # see issue#52
-        app.logger.critical("FOO")
         sub_q = filter_release_status(sub_q, status="SUCCESSFUL")
 
     sub_q = sub_q \
diff --git a/orlo/routes/info.py b/orlo/routes/info.py
index 30470bd..4cf5c37 100644
--- a/orlo/routes/info.py
+++ b/orlo/routes/info.py
@@ -1,6 +1,8 @@
 from __future__ import print_function
 from flask import request, jsonify, url_for
 from orlo.app import app
+from orlo.util import str_to_bool
+from orlo.config import config
 import orlo.queries as queries
 
 __author__ = 'alforbes'
@@ -116,10 +118,22 @@ def info_package_versions():
     """
     Return current version of all packages
 
-    :query platform:
+    :query bool platform: Filter versions by platform
+    :query bool by_release: If true, a package, that is part 
+        of a release which is not SUCCESSFUL, will not be considered the 
+        current version, even if its own status is SUCCESSFUL.
+        If false, a package will be the current version as long as its own 
+        status is SUCCESSFUL. Default: False.
     """
     platform = request.args.get('platform')
-    q = queries.package_versions(platform=platform)
+    exclude_partial_releases = str_to_bool(
+        request.args.get('by_release') or \
+        config.get('behaviour', 'versions_by_release')
+    )
+
+    q = queries.package_versions(
+        platform=platform,
+        by_release=exclude_partial_releases)
     result = q.all()
 
     packages = {}
diff --git a/tests/test_queries.py b/tests/test_queries.py
index ce9f182..4b0aaee 100644
--- a/tests/test_queries.py
+++ b/tests/test_queries.py
@@ -364,7 +364,7 @@ def test_package_versions_excludes_partial_releases(self):
         # note - have not stopped packageTwo
 
         result = orlo.queries.package_versions(
-            exclude_partial_releases=True, platform='platformOne').all()
+            by_release=True, platform='platformOne').all()
         self.assertEqual(len(result), 2)
         for pkg, ver in result:
             # Should both be version 1.0, despite packageOne 2.0 being
diff --git a/tests/test_route_info.py b/tests/test_route_info.py
index cc7c44b..98f4732 100644
--- a/tests/test_route_info.py
+++ b/tests/test_route_info.py
@@ -162,3 +162,9 @@ def test_info_package_versions_with_platform_negative(self):
         response = self.client.get('/info/packages/versions?platform=non-existent-platform')
         self.assert200(response)
         self.assertNotIn('test-package', response.json)
+
+    def test_info_package_versions_with_by_release(self):
+        self._create_finished_release()
+        response = self.client.get('/info/packages/versions?by_release=true')
+        self.assert200(response)
+        self.assertIn('test-package', response.json)

From 18d8b53c3128c8df44922f534cf54b0391895b1a Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Wed, 17 May 2017 16:03:22 +0100
Subject: [PATCH 19/22] Bump orloclient version requirement

---
 setup.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/setup.py b/setup.py
index 0595574..ade9fcd 100755
--- a/setup.py
+++ b/setup.py
@@ -26,7 +26,7 @@
     'Flask-TokenAuth',
     'arrow',
     'gunicorn',
-    'orloclient>=0.2.0',
+    'orloclient>=0.4.5',
     'psycopg2',
     'pyldap',
     'pytz',

From 6b654ae96e85e16c33871d227964eb28dacb1842 Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Wed, 17 May 2017 16:04:00 +0100
Subject: [PATCH 20/22] Bump version to 0.4.3

---
 docs/conf.py | 2 +-
 setup.py     | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/docs/conf.py b/docs/conf.py
index dc298cc..59e3b59 100644
--- a/docs/conf.py
+++ b/docs/conf.py
@@ -64,7 +64,7 @@
 # The short X.Y version.
 version = '0.4'
 # The full version, including alpha/beta/rc tags.
-release = '0.4.1'
+release = '0.4.3'
 
 # The language for content autogenerated by Sphinx. Refer to documentation
 # for a list of supported languages.
diff --git a/setup.py b/setup.py
index ade9fcd..08028a5 100755
--- a/setup.py
+++ b/setup.py
@@ -8,7 +8,7 @@
 
 __location__ = os.path.realpath(os.path.join(os.getcwd(), os.path.dirname(__file__)))
 
-VERSION = '0.4.2'
+VERSION = '0.4.3'
 
 version_file = open(os.path.join(__location__, 'orlo', '_version.py'), 'w')
 version_file.write("__version__ = '{}'".format(VERSION))

From 2c4e94d58376c7b062315c0af8d6469d9ac804ea Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Wed, 17 May 2017 16:26:25 +0100
Subject: [PATCH 21/22] Fix pylint problems

---
 orlo/routes/info.py          |  9 ++++-----
 tests/test_queries.py        | 20 ++++++--------------
 tests/test_route_releases.py | 11 ++++++-----
 3 files changed, 16 insertions(+), 24 deletions(-)

diff --git a/orlo/routes/info.py b/orlo/routes/info.py
index 4cf5c37..0e80637 100644
--- a/orlo/routes/info.py
+++ b/orlo/routes/info.py
@@ -119,11 +119,10 @@ def info_package_versions():
     Return current version of all packages
 
     :query bool platform: Filter versions by platform
-    :query bool by_release: If true, a package, that is part 
-        of a release which is not SUCCESSFUL, will not be considered the 
-        current version, even if its own status is SUCCESSFUL.
-        If false, a package will be the current version as long as its own 
-        status is SUCCESSFUL. Default: False.
+    :query bool by_release: If true, a package that is part of a release, which
+        is not SUCCESSFUL, will not be considered the current version, even if
+        its own status is SUCCESSFUL. If false, a package will be the current
+        version as long as its own status is SUCCESSFUL. Default: False.
     """
     platform = request.args.get('platform')
     exclude_partial_releases = str_to_bool(
diff --git a/tests/test_queries.py b/tests/test_queries.py
index 4b0aaee..3fd50ea 100644
--- a/tests/test_queries.py
+++ b/tests/test_queries.py
@@ -1,15 +1,12 @@
 from __future__ import print_function, unicode_literals
 import arrow
-import json
 from test_orm import OrloDbTest
 import orlo.queries
 import orlo.exceptions
 import orlo.stats
-from orlo.orm import Release, Package, app
+from orlo.orm import Release, Package
 from time import sleep
 import sqlalchemy.orm
-import uuid
-from orlo.util import is_uuid
 import logging
 
 __author__ = 'alforbes'
@@ -20,11 +17,6 @@
 
 
 
-"""
-Test the query functions in queries.py
-"""
-
-
 class OrloQueryTest(OrloDbTest):
     pass
 
@@ -274,9 +266,9 @@ def test_package_versions(self):
         """
         Test package_versions
 
-        In this test, we create two releases. packageOne succeeds in both but 
-        packageTwo fails in the second, therefore the current version for 
-        packageOne should be the second release, but packageTwo should remain 
+        In this test, we create two releases. packageOne succeeds in both but
+        packageTwo fails in the second, therefore the current version for
+        packageOne should be the second release, but packageTwo should remain
         with the first version
         """
         rid1 = self._create_release(platforms=['platformOne'])
@@ -341,7 +333,7 @@ def test_package_versions_with_rollback(self):
 
     def test_package_versions_excludes_partial_releases(self):
         """ Test that we exclude releases in progress
-        
+
         When partial_releases is False (the default). See issue#52.
         """
         # Full release of both components
@@ -366,7 +358,7 @@ def test_package_versions_excludes_partial_releases(self):
         result = orlo.queries.package_versions(
             by_release=True, platform='platformOne').all()
         self.assertEqual(len(result), 2)
-        for pkg, ver in result:
+        for _, ver in result:
             # Should both be version 1.0, despite packageOne 2.0 being
             # successful
             self.assertEqual(ver, '1.0')
diff --git a/tests/test_route_releases.py b/tests/test_route_releases.py
index 72e6d5e..be2a462 100644
--- a/tests/test_route_releases.py
+++ b/tests/test_route_releases.py
@@ -179,8 +179,8 @@ class TestGetContract(OrloHttpTest):
 
     def _get_releases(self, release_id=None, filters=None, expected_status=200):
         """
-        Perform a GET to /releases with optional filters
-        
+        Perform a GET to /releases with optional
+
         :param string release_id: release uuid
         :param list filters: list of filters
         :param expected_status: status code to expect (or None to not assert)
@@ -227,14 +227,15 @@ def test_get_single_release_404(self):
         """
         Test that we return 404 with a valid but non-existent release ID
         """
-        r = self._get_releases(release_id=str(uuid.uuid4()),
-                               expected_status=404)
+        self._get_releases(release_id=str(uuid.uuid4()),
+                           expected_status=404)
 
     def test_get_filtered_releases_404(self):
         """
         Test that we return 404 with a filter that returns no results
         """
-        results = self._get_releases(expected_status=404,
+        self._get_releases(
+            expected_status=404,
             filters=['package_name=non-existent-package'])
 
     def test_get_releases(self):

From 56c74d8b65911e48b90c769cd104b1f87bee660a Mon Sep 17 00:00:00 2001
From: Alex Forbes <github@al4.co.nz>
Date: Wed, 17 May 2017 16:34:29 +0100
Subject: [PATCH 22/22] Fix some pylint issues in stats.py

---
 orlo/stats.py | 55 +++++++++++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 24 deletions(-)

diff --git a/orlo/stats.py b/orlo/stats.py
index eb1ff2b..c9817e4 100644
--- a/orlo/stats.py
+++ b/orlo/stats.py
@@ -1,9 +1,9 @@
 from __future__ import print_function
+from collections import OrderedDict
 from orlo.queries import apply_filters, filter_release_rollback, filter_release_status
 from orlo.app import app
-from orlo.orm import db, Release, Platform, Package, release_platform
-from orlo.exceptions import OrloError, InvalidUsage
-from collections import OrderedDict
+from orlo.orm import db, Release, Package
+from orlo.exceptions import InvalidUsage
 
 __author__ = 'alforbes'
 
@@ -20,7 +20,9 @@ def releases_by_time(unit, summarize_by_unit=False, **kwargs):
     :param unit: Passed to add_release_by_time_to_dict()
     """
 
-    query = db.session.query(Release.id, Release.stime).join(Package).group_by(Release)
+    query = db.session.query(Release.id, Release.stime)\
+        .join(Package)\
+        .group_by(Release)
     query = apply_filters(query, kwargs)
 
     return get_dict_of_objects_by_time(query, unit, summarize_by_unit)
@@ -34,7 +36,8 @@ def packages_by_time(unit, summarize_by_unit=False, **kwargs):
     :param unit: Passed to add_release_by_time_to_dict()
     """
 
-    query = db.session.query(Package.id, Package.name, Package.stime).join(Release)
+    query = db.session.query(Package.id, Package.name, Package.stime)\
+        .join(Release)
     query = apply_filters(query, kwargs)
 
     return get_dict_of_objects_by_time(query, unit, summarize_by_unit)
@@ -56,29 +59,32 @@ def get_dict_of_objects_by_time(query, unit, summarize_by_unit=False):
 
     # Build queries for the individual stats
     q_normal_successful = filter_release_status(
-            filter_release_rollback(query, rollback=False), 'SUCCESSFUL'
+        filter_release_rollback(query, rollback=False), 'SUCCESSFUL'
     )
     q_normal_failed = filter_release_status(
-            filter_release_rollback(query, rollback=False), 'FAILED'
+        filter_release_rollback(query, rollback=False), 'FAILED'
     )
     q_rollback_successful = filter_release_status(
-            filter_release_rollback(query, rollback=True), 'SUCCESSFUL'
+        filter_release_rollback(query, rollback=True), 'SUCCESSFUL'
     )
     q_rollback_failed = filter_release_status(
-            filter_release_rollback(query, rollback=True), 'FAILED'
+        filter_release_rollback(query, rollback=True), 'FAILED'
     )
 
     output_dict = OrderedDict()
 
     add_objects_by_time_to_dict(
-            q_normal_successful, output_dict, ('normal', 'successful'), unit, summarize_by_unit)
+        q_normal_successful, output_dict, ('normal', 'successful'), unit,
+        summarize_by_unit)
     add_objects_by_time_to_dict(
-            q_normal_failed, output_dict, ('normal', 'failed'), unit, summarize_by_unit)
+        q_normal_failed, output_dict, ('normal', 'failed'), unit,
+        summarize_by_unit)
     add_objects_by_time_to_dict(
-            q_rollback_successful, output_dict, ('rollback', 'successful'), unit,
-            summarize_by_unit)
+        q_rollback_successful, output_dict, ('rollback', 'successful'), unit,
+        summarize_by_unit)
     add_objects_by_time_to_dict(
-            q_rollback_failed, output_dict, ('rollback', 'failed'), unit, summarize_by_unit)
+        q_rollback_failed, output_dict, ('rollback', 'failed'), unit,
+        summarize_by_unit)
 
     return output_dict
 
@@ -86,18 +92,19 @@ def get_dict_of_objects_by_time(query, unit, summarize_by_unit=False):
 def add_objects_by_time_to_dict(query, releases_dict, t_category, unit='month',
                                 summarize_by_unit=False):
     """
-    Take a query and add each of its objects to a dictionary, broken down by time
+    Take a query and add its objects to a dictionary, broken down by time
 
-    If the query given has a 'name' column, that will be included in the dictionary path
-    above the categories (t_category).
+    If the query given has a 'name' column, that will be included in the
+    dictionary path above the categories (t_category).
 
     :param dict releases_dict: Dict to add to
-    :param tuple t_category: tuple of headings, i.e. (<normal|rollback>, <successful|failed>)
+    :param tuple t_category: tuple of headings, i.e. (<normal|rollback>,
+        <successful|failed>)
     :param query query: Query object to retrieve releases from
     :param string unit: Can be 'iso', 'hour', 'day', 'week', 'month', 'year',
-    :param boolean summarize_by_unit: Only break down releases by the given unit, i.e. only one
-        layer deep. For example, if "year" is the unit, we group all releases under the year
-        and do not add month etc underneath.
+    :param boolean summarize_by_unit: Only break down releases by the given
+        unit, i.e. only one layer deep. For example, if "year" is the unit, we
+        group all releases under the year and do not add month etc underneath.
     :return:
 
     **Note**: this can also be use for packages
@@ -124,9 +131,9 @@ def add_objects_by_time_to_dict(query, releases_dict, t_category, unit='month',
                              str(object_.stime.day), str(object_.stime.hour)]
             else:
                 raise InvalidUsage(
-                        'Invalid unit "{}" specified for release breakdown'.format(unit))
+                    'Invalid unit "{}" specified for release breakdown'.format(
+                        unit))
         if hasattr(object_, 'name'):
-            #
             tree_args.append(object_.name)
         # Append categories
         tree_args += t_category
@@ -144,7 +151,7 @@ def append_tree_recursive(tree, parent, nodes, node_index=0):
     :return:
     """
     app.logger.debug('Called recursive function with args:\n{}, {}, {}'.format(
-            str(tree), str(parent), str(nodes)))
+        str(tree), str(parent), str(nodes)))
 
     child_index = node_index + 1
     try: