From 633360f56580699a7bd60ee9894c277faa63fa00 Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Tue, 5 Jan 2016 18:15:33 +0000 Subject: [PATCH 01/18] Change /info/user endpoint platform now a query param rather than path, added username to path instead. Feels more logical. --- orlo/queries.py | 15 +++++++++++++++ orlo/route_info.py | 15 +++++++++++---- tests/test_route_info.py | 13 +++++++++++-- 3 files changed, 37 insertions(+), 6 deletions(-) diff --git a/orlo/queries.py b/orlo/queries.py index 011473e..1cafac3 100644 --- a/orlo/queries.py +++ b/orlo/queries.py @@ -25,6 +25,21 @@ def user_summary(platform=None): return query.group_by(Release.user) +def user_info(username): + """ + Get user info for a single user + + :param username: + :return: + """ + query = db.session.query( + Release.user, db.func.count(Release.id))\ + .filter(Release.user == username)\ + .group_by(Release.user) + + return query + + def user_list(platform=None): """ Find all users that have performed releases diff --git a/orlo/route_info.py b/orlo/route_info.py index 9e2b2c5..cdedede 100644 --- a/orlo/route_info.py +++ b/orlo/route_info.py @@ -30,14 +30,21 @@ def info_root(): @app.route('/info/users', methods=['GET']) -@app.route('/info/users/', methods=['GET']) -def info_users(platform=None): +@app.route('/info/users/', methods=['GET']) +def info_users(username=None): """ Return a dictionary of users optionally filtering by platform - :param platform: + :param username: Username to get info for + :query platform: Platform to filter on """ - users = queries.user_summary(platform) + platform = request.args.get('platform') + + if username: + users = queries.user_info(username) + else: + users = queries.user_summary(platform) + d = {} for user, count in users: d[user] = {'releases': count} diff --git a/tests/test_route_info.py b/tests/test_route_info.py index fd1e3c1..7bc881a 100644 --- a/tests/test_route_info.py +++ b/tests/test_route_info.py @@ -22,12 +22,21 @@ def test_info_users(self): self.assert200(response) self.assertIn('userOne', response.json) + def test_info_users_with_user(self): + """ + Test /info/users/username returns 200 + """ + self._create_release(user='userOne') + response = self.client.get('/info/users/userOne') + self.assert200(response) + self.assertIn('userOne', response.json) + def test_info_users_with_platform(self): """ - Test /info/ returns 200 + Test /info?platform= returns 200 """ self._create_release(user='userOne', platforms=['platformOne']) - response = self.client.get('/info/users/platformOne') + response = self.client.get('/info/users?platform=platformOne') self.assert200(response) self.assertIn('userOne', response.json) From 7a03be0a4f5b82c7927f94179b3ca7b8f600d430 Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Tue, 5 Jan 2016 18:23:50 +0000 Subject: [PATCH 02/18] Remove print statement, fix minor documentation bugs in /info --- orlo/route_info.py | 3 +++ tests/test_route_info.py | 1 - 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/orlo/route_info.py b/orlo/route_info.py index cdedede..d808db5 100644 --- a/orlo/route_info.py +++ b/orlo/route_info.py @@ -69,6 +69,7 @@ def info_platforms(): def info_packages(platform=None): """ Summary of packages + :param platform: """ q = queries.package_summary(platform=platform) @@ -85,6 +86,7 @@ def info_packages(platform=None): def info_package_list(platform=None): """ Return list of all known packages + :param platform: """ q = queries.package_list(platform=platform) @@ -98,6 +100,7 @@ def info_package_list(platform=None): def info_package_versions(platform=None): """ Return current version of all packages + :param platform: """ q = queries.package_versions(platform=platform) diff --git a/tests/test_route_info.py b/tests/test_route_info.py index 7bc881a..16d6a41 100644 --- a/tests/test_route_info.py +++ b/tests/test_route_info.py @@ -11,7 +11,6 @@ def test_info_root(self): """ response = self.client.get('/info') self.assert200(response) - print(response.json) def test_info_users(self): """ From 6da4a917c73575fef09ce57d383d2c7f13f36387 Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Wed, 6 Jan 2016 13:51:23 +0000 Subject: [PATCH 03/18] Move platform in /info routes to query parameter Having it in the url didn't make much sense. Makes room for e.g. /info/user/ --- orlo/queries.py | 51 ++++++++++++++++++++++++-- orlo/route_info.py | 45 +++++++++++++++-------- tests/test_route_info.py | 78 +++++++++++++++++++++++++++++++++++++--- 3 files changed, 152 insertions(+), 22 deletions(-) diff --git a/orlo/queries.py b/orlo/queries.py index 1cafac3..1df5152 100644 --- a/orlo/queries.py +++ b/orlo/queries.py @@ -68,6 +68,21 @@ def team_summary(platform=None): return query.group_by(Release.team) +def team_info(team_name): + """ + Get info for a single team + + :param team_name: + :return: + """ + query = db.session.query( + Release.user, db.func.count(Release.id))\ + .filter(Release.team == team_name)\ + .group_by(Release.team) + + return query + + def team_list(platform=None): """ Find all teams that have performed releases @@ -83,17 +98,18 @@ def team_list(platform=None): def package_summary(platform=None, stime=None, ftime=None): """ - Summary of releases by package + Summary of packages :param stime: Start time, or time lower bound :param ftime: Finish time, or time upper bound - :param platform: + :param platform: Filter by platform """ query = db.session.query(Package.name, db.func.count(Package.release_id)) if any(x is not None for x in [platform, stime, ftime]): query = query.join(Release) if platform: + print("Platform filter") query = query.filter(Release.platforms.any(Platform.name == platform)) if stime: query = query.filter(Release.stime > stime) @@ -105,6 +121,21 @@ def package_summary(platform=None, stime=None, ftime=None): return query +def package_info(package_name): + """ + Return a query for a package and how many times it was released + + :param package_name: + :return: + """ + query = db.session.query( + Package.name, db.func.count(Package.id))\ + .filter(Package.name == package_name)\ + .group_by(Package.name) + + return query + + def package_list(platform=None): """ Find all packages that have been released @@ -277,6 +308,22 @@ def platform_summary(): return query +def platform_info(platform_name): + """ + Return a single platform and how many times it was released + + :param platform_name: + :return: + """ + + query = db.session.query( + Platform.name, db.func.count(Platform.id)) \ + .filter(Platform.name == platform_name)\ + .group_by(Platform.name) + + return query + + def platform_list(): """ Return a summary of the known platforms diff --git a/orlo/route_info.py b/orlo/route_info.py index d808db5..97647a4 100644 --- a/orlo/route_info.py +++ b/orlo/route_info.py @@ -53,42 +53,57 @@ def info_users(username=None): @app.route('/info/platforms', methods=['GET']) -def info_platforms(): +@app.route('/info/platforms/', methods=['GET']) +def info_platforms(platform=None): """ Return a summary of the platforms + + :param platform: Platform to get info for """ + if platform: + platforms = queries.platform_info(platform) + else: + platforms = queries.platform_summary() + d = {} - for platform, count in queries.platform_summary(): + for platform, count in platforms: d[platform] = {'releases': count} return jsonify(d), 200 @app.route('/info/packages', methods=['GET']) -@app.route('/info/packages/', methods=['GET']) -def info_packages(platform=None): +@app.route('/info/packages/', methods=['GET']) +def info_packages(package=None): """ Summary of packages - :param platform: + :param package: Package to get info for + :query platform: Platform to filter on """ - q = queries.package_summary(platform=platform) + platform = request.args.get('platform') - packages = {} + if package: + packages = queries.package_info(package) + else: + packages = queries.package_summary(platform=platform) - for package, count in q.all(): - packages[package] = {'releases': count} + d = {} + + for package, count in packages: + d[package] = {'releases': count} return jsonify(packages), 200 @app.route('/info/packages/list', methods=['GET']) -@app.route('/info/packages/list/', methods=['GET']) -def info_package_list(platform=None): +def info_package_list(): """ Return list of all known packages - :param platform: + :query platform: Platform to filter on """ + + platform = request.args.get('platform') q = queries.package_list(platform=platform) result = q.all() packages = [r[0] for r in result] @@ -96,13 +111,13 @@ def info_package_list(platform=None): @app.route('/info/packages/versions', methods=['GET']) -@app.route('/info/packages/versions/', methods=['GET']) -def info_package_versions(platform=None): +def info_package_versions(): """ Return current version of all packages - :param platform: + :query platform: """ + platform = request.args.get('platform') q = queries.package_versions(platform=platform) result = q.all() diff --git a/tests/test_route_info.py b/tests/test_route_info.py index 16d6a41..84f6372 100644 --- a/tests/test_route_info.py +++ b/tests/test_route_info.py @@ -48,6 +48,24 @@ def test_info_platforms(self): self.assert200(response) self.assertIn('platformOne', response.json) + def test_info_platforms_with_platform(self): + """ + Test /info/platforms/ returns 200 + """ + self._create_release(platforms=['platformOne']) + response = self.client.get('/info/platforms/platformOne') + self.assert200(response) + self.assertIn('platformOne', response.json) + + def test_info_platforms_with_platform_negative(self): + """ + Test /info/platforms/ returns 200 + """ + self._create_release(platforms=['platformOne']) + response = self.client.get('/info/platforms/badPlatform') + self.assert200(response) + self.assertNotIn('platformOne', response.json) + def test_info_packages(self): """ Test /info/packages @@ -58,7 +76,37 @@ def test_info_packages(self): self.assert200(response) self.assertIn('test-package', response.json) - def test_info_package_list(self): + def test_info_packages_with_package(self): + """ + Test /info/package with a package + """ + rid = self._create_release() + self._create_package(rid, name='packageOne') + response = self.client.get('/info/packages/packageOne') + self.assert200(response) + self.assertIn('packageOne', response.json) + + def test_info_packages_with_platform(self): + """ + Test /info/package with a platform filter + """ + rid = self._create_release(platforms=['platformOne']) + self._create_package(rid, name='packageOne') + response = self.client.get('/info/packages?platform=platformOne') + self.assert200(response) + self.assertIn('packageOne', response.json) + + def test_info_packages_with_platform_negative(self): + """ + Test /info/package with a platform filter + """ + rid = self._create_release(platforms=['platformOne']) + self._create_package(rid, name='packageOne') + response = self.client.get('/info/packages?platform=platformFoo') + self.assert200(response) + self.assertNotIn('packageOne', response.json) + + def test_info_packages_list(self): """ Test /info/package_list """ @@ -68,7 +116,27 @@ def test_info_package_list(self): self.assert200(response) self.assertIn('test-package', response.json['packages']) - def test_info_package_versions(self): + def test_info_packages_list_with_platform(self): + """ + Test /info/package_list + """ + + self._create_finished_release() + response = self.client.get('/info/packages/list?platform=test_platform') + self.assert200(response) + self.assertIn('test-package', response.json['packages']) + + def test_info_packages_list_with_platform_negative(self): + """ + Test /info/package_list + """ + + self._create_finished_release() + response = self.client.get('/info/packages/list?platform=non-existent-platform') + self.assert200(response) + self.assertNotIn('test-package', response.json['packages']) + + def test_info_packages_versions(self): """ Test /info/packages returns 200 """ @@ -77,12 +145,12 @@ def test_info_package_versions(self): self.assert200(response) self.assertIn('test-package', response.json) - def test_info_package_versions_with_platform(self): + def test_info_packages_versions_with_platform(self): """ Test /info/packages returns 200 """ self._create_finished_release() - response = self.client.get('/info/packages/versions/test_platform') + response = self.client.get('/info/packages/versions?platform=test_platform') self.assert200(response) self.assertIn('test-package', response.json) @@ -91,6 +159,6 @@ def test_info_package_versions_with_platform_negative(self): Test /info/packages returns 200 """ self._create_finished_release() - response = self.client.get('/info/packages/versions/non-existent-platform') + response = self.client.get('/info/packages/versions?platform=non-existent-platform') self.assert200(response) self.assertNotIn('test-package', response.json) From 1b3930bae4aa37eb51aed33a97c088e576cb5a99 Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Wed, 6 Jan 2016 13:52:15 +0000 Subject: [PATCH 04/18] Stream output of GET /releases to reduce memory usage The output of /releases can get rather large, but jsonify doesn't support streaming. This adds a lagging generator which should always result in valid json --- orlo/route_api.py | 32 ++++++++++++++++++++++++-------- setup.py | 2 +- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/orlo/route_api.py b/orlo/route_api.py index 2d5bcfe..fb548e2 100644 --- a/orlo/route_api.py +++ b/orlo/route_api.py @@ -1,6 +1,6 @@ from orlo import app from orlo.exceptions import InvalidUsage -from flask import jsonify, request +from flask import jsonify, request, Response, json import arrow import datetime from orlo.orm import db, Release, Package, PackageResult, ReleaseNote, Platform @@ -282,14 +282,30 @@ def get_releases(release_id=None): if request.args.get('latest'): query = query.limit(1) - app.logger.debug("Query: {}".format(str(query))) - releases = query.all() + def generate(): + """ + A lagging generator to stream JSON so we don't have to hold everything in memory - app.logger.debug("Returning {} releases".format(len(releases))) - output = [] - for r in releases: - output.append(r.to_dict()) + This is a little tricky, as we need to omit the last comma to make valid JSON, + thus we use a lagging generator, similar to http://stackoverflow.com/questions/1630320/ + """ + releases = query.__iter__() + try: + prev_release = next(releases) # get first result + except StopIteration: + # StopIteration here means the length was zero, so yield a valid releases doc and stop + yield '{"releases": []}' + raise StopIteration + + # We have some releases. First, yield the opening json + yield '{"releases": [' - return jsonify(releases=output), 200 + # Iterate over the releases + for release in releases: + yield json.dumps(prev_release.to_dict()) + ', ' + prev_release = release + # Now yield the last iteration without comma but with the closing brackets + yield json.dumps(prev_release.to_dict()) + ']}' + return Response(generate(), content_type='application/json') diff --git a/setup.py b/setup.py index 5da3d6a..590cba7 100755 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ import multiprocessing # nopep8 -VERSION = '0.0.5-7' +VERSION = '0.0.5-8' version_file = open('./orlo/_version.py', 'w') version_file.write("__version__ = '{}'".format(VERSION)) version_file.close() From 678f77f0b8a284325d766e648300b1f23eeb4593 Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Wed, 6 Jan 2016 16:14:32 +0000 Subject: [PATCH 05/18] Move /releases streaming json generator to util.py --- orlo/queries.py | 1 - orlo/route_api.py | 31 +++---------------------------- orlo/util.py | 32 ++++++++++++++++++++++++++++++++ setup.py | 2 +- tests/test_contract.py | 1 - 5 files changed, 36 insertions(+), 31 deletions(-) diff --git a/orlo/queries.py b/orlo/queries.py index 1df5152..bed8f63 100644 --- a/orlo/queries.py +++ b/orlo/queries.py @@ -109,7 +109,6 @@ def package_summary(platform=None, stime=None, ftime=None): if any(x is not None for x in [platform, stime, ftime]): query = query.join(Release) if platform: - print("Platform filter") query = query.filter(Release.platforms.any(Platform.name == platform)) if stime: query = query.filter(Release.stime > stime) diff --git a/orlo/route_api.py b/orlo/route_api.py index fb548e2..1cfded2 100644 --- a/orlo/route_api.py +++ b/orlo/route_api.py @@ -5,7 +5,8 @@ import datetime from orlo.orm import db, Release, Package, PackageResult, ReleaseNote, Platform from orlo.util import validate_request_json, create_release, validate_release_input, \ - validate_package_input, fetch_release, create_package, fetch_package, apply_filters + validate_package_input, fetch_release, create_package, fetch_package, apply_filters, \ + stream_json_list @app.route('/ping', methods=['GET']) @@ -282,30 +283,4 @@ def get_releases(release_id=None): if request.args.get('latest'): query = query.limit(1) - def generate(): - """ - A lagging generator to stream JSON so we don't have to hold everything in memory - - This is a little tricky, as we need to omit the last comma to make valid JSON, - thus we use a lagging generator, similar to http://stackoverflow.com/questions/1630320/ - """ - releases = query.__iter__() - try: - prev_release = next(releases) # get first result - except StopIteration: - # StopIteration here means the length was zero, so yield a valid releases doc and stop - yield '{"releases": []}' - raise StopIteration - - # We have some releases. First, yield the opening json - yield '{"releases": [' - - # Iterate over the releases - for release in releases: - yield json.dumps(prev_release.to_dict()) + ', ' - prev_release = release - - # Now yield the last iteration without comma but with the closing brackets - yield json.dumps(prev_release.to_dict()) + ']}' - - return Response(generate(), content_type='application/json') + return Response(stream_json_list('releases', query), content_type='application/json') diff --git a/orlo/util.py b/orlo/util.py index 6150bef..f1c5508 100644 --- a/orlo/util.py +++ b/orlo/util.py @@ -1,6 +1,7 @@ from __future__ import print_function, unicode_literals import arrow import datetime +from flask import json from orlo import app from orlo.orm import db, Release, Package, Platform from orlo.exceptions import InvalidUsage @@ -224,3 +225,34 @@ def apply_filters(query, args): query = query.filter(filter_field.any(sub_field == value)) return query + + +def stream_json_list(heading, iterator): + """ + A lagging generator to stream JSON so we don't have to hold everything in memory + + This is a little tricky, as we need to omit the last comma to make valid JSON, + thus we use a lagging generator, similar to http://stackoverflow.com/questions/1630320/ + + :param heading: The title of the set, e.g. "releases" + :param iterator: Any object with __iter__(), e.g. SQLAlchemy Query + """ + iterator = iterator.__iter__() + try: + prev_release = next(iterator) # get first result + except StopIteration: + # StopIteration here means the length was zero, so yield a valid releases doc and stop + yield '{{"{}": []}}'.format(heading) + raise StopIteration + + # We have some releases. First, yield the opening json + yield '{{"{}": ['.format(heading) + + # Iterate over the releases + for item in iterator: + yield json.dumps(prev_release.to_dict()) + ', ' + prev_release = item + + # Now yield the last iteration without comma but with the closing brackets + yield json.dumps(prev_release.to_dict()) + ']}' + diff --git a/setup.py b/setup.py index 590cba7..c766d51 100755 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ import multiprocessing # nopep8 -VERSION = '0.0.5-8' +VERSION = '0.0.5-9' version_file = open('./orlo/_version.py', 'w') version_file.write("__version__ = '{}'".format(VERSION)) version_file.close() diff --git a/tests/test_contract.py b/tests/test_contract.py index 8607076..f4add96 100644 --- a/tests/test_contract.py +++ b/tests/test_contract.py @@ -300,7 +300,6 @@ def _get_releases(self, release_id=None, filters=None, expected_status=200): else: path = '/releases' - print("GET {}".format(path)) results_response = self.client.get( path, content_type='application/json', ) From 87c582b8880259a0375085bba741008d42190156 Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Wed, 6 Jan 2016 17:57:52 +0000 Subject: [PATCH 06/18] Add example curl to documentation for /import --- orlo/route_import.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/orlo/route_import.py b/orlo/route_import.py index e93c088..1fc9f51 100644 --- a/orlo/route_import.py +++ b/orlo/route_import.py @@ -74,6 +74,13 @@ def post_import(): A json null value is acceptable for non-required fields, or it can be omitted entirely. See `orlo.orm.Release` and `orlo.orm.Package`. + **Example curl**: + + .. sourcecode:: shell + + curl -v -X POST -d @releases.json 'http://127.0.0.1:5000/releases/import' -H \ + "Content-Type: application/json" + :status 200: The document was accepted """ From 2b8c68ae24b6cc0261c401d092e6f8a6ddba6056 Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Wed, 6 Jan 2016 19:03:45 +0000 Subject: [PATCH 07/18] Abstract release logic away from the get_releases route --- orlo/queries.py | 114 +++++++++++++++++++++++++++++++++++++++++++++- orlo/route_api.py | 38 ++++++---------- orlo/util.py | 79 -------------------------------- 3 files changed, 126 insertions(+), 105 deletions(-) diff --git a/orlo/queries.py b/orlo/queries.py index bed8f63..12c29dc 100644 --- a/orlo/queries.py +++ b/orlo/queries.py @@ -1,7 +1,9 @@ from __future__ import print_function +import datetime +import arrow from orlo import app from orlo.orm import db, Release, Platform, Package, release_platform -from orlo.exceptions import OrloError +from orlo.exceptions import OrloError, InvalidUsage __author__ = 'alforbes' @@ -10,6 +12,114 @@ """ +def apply_filters(query, args): + """ + Apply filters to a query + + :param query: Query object to apply filters to + :param args: Dictionary of arguments, usually request.args + + :return: filtered query object + """ + + for field, value in args.iteritems(): + if field == 'latest': # this is not a comparison + continue + + if field.startswith('package_'): + # Package attribute. Ensure source query does a join on Package. + db_table = Package + field = '_'.join(field.split('_')[1:]) + else: + db_table = Release + + comparison = '==' + time_absolute = False + time_delta = False + strip_last = False + sub_field = None + + if field.endswith('_gt'): + strip_last = True + comparison = '>' + if field.endswith('_lt'): + strip_last = True + comparison = '<' + if field.endswith('_before'): + strip_last = True + comparison = '<' + time_absolute = True + if field.endswith('_after'): + strip_last = True + comparison = '>' + time_absolute = True + if 'duration' in field.split('_'): + time_delta = True + if field == 'platform': + field = 'platforms' + comparison = 'any' + sub_field = Platform.name + + if strip_last: + # Strip anything after the last underscore inclusive + field = '_'.join(field.split('_')[:-1]) + + filter_field = getattr(db_table, field) + + # Booleans + if value in ('True', 'true'): + value = True + if value in ('False', 'false'): + value = False + + # Time related + if time_delta: + value = datetime.timedelta(seconds=int(value)) + if time_absolute: + value = arrow.get(value) + + # Do comparisons + app.logger.debug("Filtering: {} {} {}".format(filter_field, comparison, value)) + if comparison == '==': + query = query.filter(filter_field == value) + if comparison == '<': + query = query.filter(filter_field < value) + if comparison == '>': + query = query.filter(filter_field > value) + if comparison == 'any': + query = query.filter(filter_field.any(sub_field == value)) + + return query + + +def releases(**kwargs): + """ + Return whole releases, based on filters + + :param kwargs: Request arguments + :return: + """ + + if any(field.startswith('package_') for field in kwargs.keys()): + query = db.session.query(Release).join(Package) + else: + # No need to join on package if none of our params need it + query = db.session.query(Release) + + try: + query = apply_filters(query, kwargs) + except AttributeError as e: + raise InvalidUsage("An invalid field was specified: {}".format(e.message)) + + if kwargs.get('latest', False): + # sort descending so first is most recent + query = query.order_by(Release.stime.desc()).limit(1) + else: # ascending + query = query.order_by(Release.stime.asc()) + + return query + + def user_summary(platform=None): """ Find all users that have performed releases and how many @@ -334,3 +444,5 @@ def platform_list(): .join(release_platform) return query + + diff --git a/orlo/route_api.py b/orlo/route_api.py index 1cfded2..77b6caf 100644 --- a/orlo/route_api.py +++ b/orlo/route_api.py @@ -1,12 +1,11 @@ -from orlo import app +from orlo import app, queries from orlo.exceptions import InvalidUsage from flask import jsonify, request, Response, json import arrow import datetime from orlo.orm import db, Release, Package, PackageResult, ReleaseNote, Platform from orlo.util import validate_request_json, create_release, validate_release_input, \ - validate_package_input, fetch_release, create_package, fetch_package, apply_filters, \ - stream_json_list + validate_package_input, fetch_release, create_package, fetch_package, stream_json_list @app.route('/ping', methods=['GET']) @@ -260,27 +259,16 @@ def get_releases(release_id=None): """ - if any(field.startswith('package_') for field in request.args.keys()): - query = db.session.query(Release).join(Package) - else: - # No need to join on package if none of our params need it - query = db.session.query(Release) - - if request.args.get('latest', False): - # sort descending so we can use .first() - query = query.order_by(Release.stime.desc()) - else: # ascending - query = query.order_by(Release.stime.asc()) - - if release_id: - query = query.filter(Release.id == release_id) - elif request.args: - try: - query = apply_filters(query, request.args) - except AttributeError as e: - raise InvalidUsage("An invalid field was specified: {}".format(e.message)) - - if request.args.get('latest'): - query = query.limit(1) + if release_id: # Simple + query = db.session.query(Release).filter(Release.id == release_id) + else: # Bit more complex + # Flatten args, as the ImmutableDict puts some values in a list when expanded + args = {} + for k, v in request.args.items(): + if type(v) is list: + args[k] = v[0] + else: + args[k] = v + query = queries.releases(**args) return Response(stream_json_list('releases', query), content_type='application/json') diff --git a/orlo/util.py b/orlo/util.py index f1c5508..8f5294b 100644 --- a/orlo/util.py +++ b/orlo/util.py @@ -147,85 +147,6 @@ def list_to_string(array): return '["' + '", "'.join(array) + '"]' -def apply_filters(query, args): - """ - Apply filters to a query - - :param query: Query object to apply filters to - :param args: Dictionary of arguments, usually request.args - - :return: filtered query object - """ - - for field, value in args.iteritems(): - if field == 'latest': # this is not a comparison - continue - - if field.startswith('package_'): - # Package attribute. Ensure source query does a join on Package. - db_table = Package - field = '_'.join(field.split('_')[1:]) - else: - db_table = Release - - comparison = '==' - time_absolute = False - time_delta = False - strip_last = False - sub_field = None - - if field.endswith('_gt'): - strip_last = True - comparison = '>' - if field.endswith('_lt'): - strip_last = True - comparison = '<' - if field.endswith('_before'): - strip_last = True - comparison = '<' - time_absolute = True - if field.endswith('_after'): - strip_last = True - comparison = '>' - time_absolute = True - if 'duration' in field.split('_'): - time_delta = True - if field == 'platform': - field = 'platforms' - comparison = 'any' - sub_field = Platform.name - - if strip_last: - # Strip anything after the last underscore inclusive - field = '_'.join(field.split('_')[:-1]) - - filter_field = getattr(db_table, field) - - # Booleans - if value in ('True', 'true'): - value = True - if value in ('False', 'false'): - value = False - - # Time related - if time_delta: - value = datetime.timedelta(seconds=int(value)) - if time_absolute: - value = arrow.get(value) - - # Do comparisons - app.logger.debug("Filtering: {} {} {}".format(filter_field, comparison, value)) - if comparison == '==': - query = query.filter(filter_field == value) - if comparison == '<': - query = query.filter(filter_field < value) - if comparison == '>': - query = query.filter(filter_field > value) - if comparison == 'any': - query = query.filter(filter_field.any(sub_field == value)) - - return query - def stream_json_list(heading, iterator): """ From 232cfeb872885b09c395250c26c7909d603efd67 Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Wed, 6 Jan 2016 19:21:09 +0000 Subject: [PATCH 08/18] Add status to get_releases parameters Allows us to filter by whole release status rather than just package --- orlo/error_handlers.py | 4 +++- orlo/exceptions.py | 1 + orlo/queries.py | 54 +++++++++++++++++++++++++++++------------- orlo/route_api.py | 8 +++++++ setup.py | 2 +- tests/test_contract.py | 35 +++++++++++++++++++++++++++ 6 files changed, 86 insertions(+), 18 deletions(-) diff --git a/orlo/error_handlers.py b/orlo/error_handlers.py index f2830cf..00d8947 100644 --- a/orlo/error_handlers.py +++ b/orlo/error_handlers.py @@ -1,7 +1,7 @@ from __future__ import print_function from flask import jsonify, request from orlo import app -from orlo.exceptions import InvalidUsage +from orlo.exceptions import InvalidUsage, OrloError __author__ = 'alforbes' @@ -12,6 +12,7 @@ def page_not_found(error): return jsonify(d), 404 +@app.errorhandler(OrloError) @app.errorhandler(InvalidUsage) def handle_invalid_usage(error): response = jsonify(error.to_dict()) @@ -25,3 +26,4 @@ def handle_400(error): response.status_code = error.status_code return response + diff --git a/orlo/exceptions.py b/orlo/exceptions.py index 1458329..73dd6c3 100644 --- a/orlo/exceptions.py +++ b/orlo/exceptions.py @@ -4,6 +4,7 @@ class OrloError(Exception): + status_code = 500 def __init__(self, message, status_code=None, payload=None): Exception.__init__(self) self.message = message diff --git a/orlo/queries.py b/orlo/queries.py index 12c29dc..d0512dc 100644 --- a/orlo/queries.py +++ b/orlo/queries.py @@ -12,6 +12,34 @@ """ +def _filter_release_status(query, status): + """ + Filter the given query by the given release status + + Release status is special, because it's actually determined by the package status + + :param query: Query object + :param status: The status to filter on + :return: + """ + enums = Package.status.property.columns[0].type.enums + if status not in enums: + raise InvalidUsage("Invalid package status, {} is not in {}".format( + status, str(enums))) + if status in ["SUCCESSFUL", "NOT_STARTED"]: + # ALL packages must match this status for it to apply to the release + # Query logic translates to "Releases which do not have any packages which satisfy + # the condition 'Package.status != status'". I.E, all match. + query = query.filter( + ~Release.packages.any( + Package.status != 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)) + return query + + def apply_filters(query, args): """ Apply filters to a query @@ -26,6 +54,11 @@ def apply_filters(query, args): if field == 'latest': # this is not a comparison continue + if field == 'status': + # special logic for this one + query = _filter_release_status(query, value) + continue + if field.startswith('package_'): # Package attribute. Ensure source query does a join on Package. db_table = Package @@ -100,7 +133,10 @@ def releases(**kwargs): :return: """ - if any(field.startswith('package_') for field in kwargs.keys()): + 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) else: # No need to join on package if none of our params need it @@ -355,21 +391,7 @@ def count_releases(user=None, package=None, team=None, platform=None, status=Non rollback, type(rollback))) if status: - enums = Package.status.property.columns[0].type.enums - if status not in enums: - raise OrloError("Invalid package status, {} is not in {}".format( - status, str(enums))) - if status in ["SUCCESSFUL", "NOT_STARTED"]: - # ALL packages must match this status for it to apply to the release - # Query logic translates to "Releases which do not have any packages which satisfy - # the condition 'Package.status != status'". I.E, all match. - query = query.filter( - ~Release.packages.any( - Package.status != 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)) + query = _filter_release_status(query, status) return query diff --git a/orlo/route_api.py b/orlo/route_api.py index 77b6caf..56a5eb1 100644 --- a/orlo/route_api.py +++ b/orlo/route_api.py @@ -244,6 +244,8 @@ def get_releases(release_id=None): :query string ftime_before: Only include releases that finished before timestamp given :query string ftime_after: Only include releases that finished after timestamp given :query string team: Filter releases by team + :query string status: Filter by release status. This field is calculated from the package. \ + status, see special note below. :query int duration_lt: Only include releases that took less than (int) seconds :query int duration_gt: Only include releases that took more than (int) seconds :query boolean package_rollback: Filter on whether or not the releases contain a rollback @@ -257,6 +259,12 @@ def get_releases(release_id=None): **Note for time arguments**: The timestamp format you must use is specified in /etc/orlo.conf. All times are UTC. + **Note on status**: + The release status is calculated from the packages it contains. The possible values are + the same as a package. For a release to be considered "SUCCESSFUL" or "NOT_STARTED", + all packages must have this value. If any one package has the value "IN_PROGRESS" or + "FAILED", that status applies to the whole release, with "FAILED" overriding "IN_PROGRESS". + """ if release_id: # Simple diff --git a/setup.py b/setup.py index c766d51..02f67f0 100755 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ import multiprocessing # nopep8 -VERSION = '0.0.5-9' +VERSION = '0.0.5-11' version_file = open('./orlo/_version.py', 'w') version_file.write("__version__ = '{}'".format(VERSION)) version_file.close() diff --git a/tests/test_contract.py b/tests/test_contract.py index f4add96..1d49471 100644 --- a/tests/test_contract.py +++ b/tests/test_contract.py @@ -693,3 +693,38 @@ def test_get_release_bad_attribute(self): r = self._get_releases(filters=['foo=bar'], expected_status=400) self.assertIn('message', r) + + def test_get_release_with_status_successful(self): + """ + Test that the release status filters correctly + """ + # A partially successful release (should be considered "FAILED") + rid1 = self._create_release() + pid1 = self._create_package(rid1, name='successful_package') + self._start_package(rid1, pid1) + self._stop_package(rid1, pid1, success=True) + pid2 = self._create_package(rid1, name='failed_package') + self._start_package(rid1, pid2) + self._stop_package(rid1, pid2, success=False) + self._stop_release(rid1) + + for _ in range(0, 2): + # These should be successful + self._create_finished_release() + + success_results = self._get_releases(filters=['status=SUCCESSFUL']) + self.assertEqual(len(success_results['releases']), 2) + + failed_results = self._get_releases(filters=['status=FAILED']) + self.assertEqual(len(failed_results['releases']), 1) + self.assertEqual(rid1, failed_results['releases'][0]['id']) + + def test_get_release_with_bad_status(self): + """ + Tests get /releases?status=garbage give a helpful mesage + """ + self._create_finished_release() + + result = self._get_releases(filters=['status=garbage_boz'], expected_status=400) + self.assertIn('message', result) + From 8ae59fbe872abb1119744952eaf9551be061ee5a Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Fri, 8 Jan 2016 11:12:52 +0000 Subject: [PATCH 09/18] Remove "latest" filter option in favour of "desc" and "limit" More flexible --- orlo/queries.py | 17 ++++++++++++----- orlo/route_api.py | 3 ++- setup.py | 2 +- tests/test_contract.py | 24 +++++++++++++++++++++--- 4 files changed, 36 insertions(+), 10 deletions(-) diff --git a/orlo/queries.py b/orlo/queries.py index d0512dc..26bd123 100644 --- a/orlo/queries.py +++ b/orlo/queries.py @@ -133,6 +133,9 @@ def releases(**kwargs): :return: """ + limit = kwargs.pop('limit', None) + desc = kwargs.pop('desc', False) + 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 @@ -147,11 +150,15 @@ def releases(**kwargs): except AttributeError as e: raise InvalidUsage("An invalid field was specified: {}".format(e.message)) - if kwargs.get('latest', False): - # sort descending so first is most recent - query = query.order_by(Release.stime.desc()).limit(1) - else: # ascending - query = query.order_by(Release.stime.asc()) + if desc: + stime_field = Release.stime.desc + else: + stime_field = Release.stime.asc + + if limit: + query = query.order_by(stime_field()).limit(limit) + else: + query = query.order_by(stime_field()) return query diff --git a/orlo/route_api.py b/orlo/route_api.py index 56a5eb1..48b2c63 100644 --- a/orlo/route_api.py +++ b/orlo/route_api.py @@ -235,7 +235,8 @@ def get_releases(release_id=None): :param string release_id: Optionally specify a single release UUID to fetch. \ This does not disable filters. - :query boolean latest: Return only the last matching release (the latest) + :query int desc: Normally results are returned ordered by stime ascending, setting + desc to true will reverse this and sort by stime descending :query string package_name: Filter releases by package name :query string user: Filter releases by user the that performed the release :query string platform: Filter releases by platform diff --git a/setup.py b/setup.py index 02f67f0..389ee99 100755 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ import multiprocessing # nopep8 -VERSION = '0.0.5-11' +VERSION = '0.0.5-12' version_file = open('./orlo/_version.py', 'w') version_file.write("__version__ = '{}'".format(VERSION)) version_file.close() diff --git a/tests/test_contract.py b/tests/test_contract.py index 1d49471..25be9df 100644 --- a/tests/test_contract.py +++ b/tests/test_contract.py @@ -303,7 +303,12 @@ def _get_releases(self, release_id=None, filters=None, expected_status=200): results_response = self.client.get( path, content_type='application/json', ) - self.assertEqual(results_response.status_code, 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) return r_json @@ -534,7 +539,7 @@ def test_get_release_filter_rollback(self): for p in r['packages']: self.assertIs(p['rollback'], False) - def test_get_release_latest(self): + def test_get_release_limit_one(self): """ Should return only one release """ @@ -544,8 +549,21 @@ def test_get_release_latest(self): rid = self._create_release() sleep(0.1) - r = self._get_releases(filters=['latest=True']) + r = self._get_releases(filters=['limit=1']) self.assertEqual(len(r['releases']), 1) + + def test_get_release_desc(self): + """ + Should return in reverse order + """ + + rid = None + for _ in range(0, 3): + rid = self._create_release() + sleep(0.1) + + r = self._get_releases(filters=['desc=true']) + # First in list should be last to be created self.assertEqual(r['releases'][0]['id'], rid) def test_get_release_package_name(self): From 84726f25a5fba1684be4a499ed342a723404de7e Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Fri, 8 Jan 2016 11:58:00 +0000 Subject: [PATCH 10/18] Rename test_import --- tests/{test_import.py => test_route_import.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename tests/{test_import.py => test_route_import.py} (100%) diff --git a/tests/test_import.py b/tests/test_route_import.py similarity index 100% rename from tests/test_import.py rename to tests/test_route_import.py From 4ef1922b355bf97b094fc4c812d5d33991866968 Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Fri, 8 Jan 2016 11:58:18 +0000 Subject: [PATCH 11/18] Add offset to get_releases --- orlo/queries.py | 9 ++++++--- orlo/route_api.py | 3 +++ tests/test_contract.py | 16 ++++++++++++++-- 3 files changed, 23 insertions(+), 5 deletions(-) diff --git a/orlo/queries.py b/orlo/queries.py index 26bd123..b1497f9 100644 --- a/orlo/queries.py +++ b/orlo/queries.py @@ -134,6 +134,7 @@ def releases(**kwargs): """ limit = kwargs.pop('limit', None) + offset = kwargs.pop('offset', None) desc = kwargs.pop('desc', False) if any(field.startswith('package_') for field in kwargs.keys())\ @@ -155,10 +156,12 @@ def releases(**kwargs): else: stime_field = Release.stime.asc + query = query.order_by(stime_field()) + if limit: - query = query.order_by(stime_field()).limit(limit) - else: - query = query.order_by(stime_field()) + query = query.limit(limit) + if offset: + query = query.offset(offset) return query diff --git a/orlo/route_api.py b/orlo/route_api.py index 48b2c63..6ab39b0 100644 --- a/orlo/route_api.py +++ b/orlo/route_api.py @@ -237,6 +237,9 @@ def get_releases(release_id=None): This does not disable filters. :query int desc: Normally results are returned ordered by stime ascending, setting desc to true will reverse this and sort by stime descending + :query int limit: Limit the results by int + :query int offset: Offset the results by int + :query int skip: Skip this number of releases :query string package_name: Filter releases by package name :query string user: Filter releases by user the that performed the release :query string platform: Filter releases by platform diff --git a/tests/test_contract.py b/tests/test_contract.py index 25be9df..d3eda7a 100644 --- a/tests/test_contract.py +++ b/tests/test_contract.py @@ -544,14 +544,26 @@ def test_get_release_limit_one(self): Should return only one release """ - rid = None for _ in range(0, 3): - rid = self._create_release() + self._create_release() sleep(0.1) r = self._get_releases(filters=['limit=1']) self.assertEqual(len(r['releases']), 1) + def test_get_release_offset(self): + """ + Test that offset=1 skips the first release + """ + rid = None + for _ in range(0, 2): + rid = self._create_release() + sleep(0.1) + + r = self._get_releases(filters=['offset=1']) + self.assertEqual(len(r['releases']), 1) + self.assertEqual(r['releases'][0]['id'], rid) + def test_get_release_desc(self): """ Should return in reverse order From f6de2d7c7573c19bb5b8ca1cec5c517d69febf0f Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Fri, 8 Jan 2016 13:33:51 +0000 Subject: [PATCH 12/18] Ensure limit and offset are ints --- orlo/queries.py | 5 +++++ orlo/util.py | 8 +++++++- setup.py | 2 +- tests/test_queries.py | 26 ++++++++++++++++++++++++++ 4 files changed, 39 insertions(+), 2 deletions(-) diff --git a/orlo/queries.py b/orlo/queries.py index b1497f9..5a7334c 100644 --- a/orlo/queries.py +++ b/orlo/queries.py @@ -4,6 +4,7 @@ from orlo import app from orlo.orm import db, Release, Platform, Package, release_platform from orlo.exceptions import OrloError, InvalidUsage +from orlo.util import is_int __author__ = 'alforbes' @@ -159,8 +160,12 @@ def releases(**kwargs): query = query.order_by(stime_field()) if limit: + if not is_int(limit): + raise InvalidUsage("limit must be a valid integer value") query = query.limit(limit) if offset: + if not is_int(offset): + raise InvalidUsage("offset must be a valid integer value") query = query.offset(offset) return query diff --git a/orlo/util.py b/orlo/util.py index 8f5294b..be892aa 100644 --- a/orlo/util.py +++ b/orlo/util.py @@ -147,7 +147,6 @@ def list_to_string(array): return '["' + '", "'.join(array) + '"]' - def stream_json_list(heading, iterator): """ A lagging generator to stream JSON so we don't have to hold everything in memory @@ -177,3 +176,10 @@ def stream_json_list(heading, iterator): # Now yield the last iteration without comma but with the closing brackets yield json.dumps(prev_release.to_dict()) + ']}' + +def is_int(value): + try: + int(value) + return True + except ValueError: + return False diff --git a/setup.py b/setup.py index 389ee99..b586103 100755 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ import multiprocessing # nopep8 -VERSION = '0.0.5-12' +VERSION = '0.0.5-14' version_file = open('./orlo/_version.py', 'w') version_file.write("__version__ = '{}'".format(VERSION)) version_file.close() diff --git a/tests/test_queries.py b/tests/test_queries.py index c407aa8..1ff73ed 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -650,3 +650,29 @@ def test_rollback_false(self): result = orlo.queries.count_packages(rollback=False).all() self.assertEqual(1, result[0][0]) + + +class ReleasesTest(OrloQueryTest): + """ + Test the releases method + """ + + def test_releases_with_bad_limit(self): + """ + Test releases raises InvalidUsage when limit is not an int + """ + args = { + 'limit': None, + } + with self.assertRaises(orlo.exceptions.InvalidUsage): + orlo.queries.releases(**args) + + def test_releases_with_bad_offset(self): + """ + Test releases raises InvalidUsage when offset is not an int + """ + args = { + 'offset': None, + } + with self.assertRaises(orlo.exceptions.InvalidUsage): + orlo.queries.releases(**args) From ed439791ae80a69f53ed56640f5146ab309e6c6f Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Fri, 8 Jan 2016 19:18:27 +0000 Subject: [PATCH 13/18] Implement time-based stats for charts --- orlo/queries.py | 180 ++++++++++++++++++++++++++++++++------ orlo/route_stats.py | 29 ++++++ setup.py | 2 +- tests/test_queries.py | 107 +++++++++++++++++++++- tests/test_route_stats.py | 49 +++++++++++ 5 files changed, 339 insertions(+), 28 deletions(-) diff --git a/orlo/queries.py b/orlo/queries.py index 5a7334c..36dba1a 100644 --- a/orlo/queries.py +++ b/orlo/queries.py @@ -1,10 +1,12 @@ from __future__ import print_function +import calendar import datetime import arrow from orlo import app from orlo.orm import db, Release, Platform, Package, release_platform from orlo.exceptions import OrloError, InvalidUsage from orlo.util import is_int +from collections import OrderedDict __author__ = 'alforbes' @@ -41,6 +43,30 @@ def _filter_release_status(query, status): return query +def _filter_release_rollback(query, rollback): + """ + Filter the given query by whether the releases are rollbacks or not + + :param query: Query object + :param boolean rollback: + :return: + """ + if rollback is True: + # Only count releases which have a rollback package + query = query.filter( + Release.packages.any(Package.rollback == True) + ) + elif rollback is False: + # Only count releases which do not have any rollback packages + query = query.filter( + ~Release.packages.any(Package.rollback == True) + ) + else: # What the hell did you pass? + raise TypeError("Bad rollback parameter: '{}', type {}. Boolean expected.".format( + rollback, type(rollback))) + return query + + def apply_filters(query, args): """ Apply filters to a query @@ -55,10 +81,13 @@ def apply_filters(query, args): if field == 'latest': # this is not a comparison continue + # special logic for these ones, as they are package attributes if field == 'status': - # special logic for this one query = _filter_release_status(query, value) continue + if field == 'rollback': + query = _filter_release_rollback(query, value) + continue if field.startswith('package_'): # Package attribute. Ensure source query does a join on Package. @@ -138,7 +167,7 @@ def releases(**kwargs): offset = kwargs.pop('offset', None) desc = kwargs.pop('desc', False) - if any(field.startswith('package_') for field in kwargs.keys())\ + 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 @@ -194,8 +223,8 @@ def user_info(username): :return: """ query = db.session.query( - Release.user, db.func.count(Release.id))\ - .filter(Release.user == username)\ + Release.user, db.func.count(Release.id)) \ + .filter(Release.user == username) \ .group_by(Release.user) return query @@ -237,8 +266,8 @@ def team_info(team_name): :return: """ query = db.session.query( - Release.user, db.func.count(Release.id))\ - .filter(Release.team == team_name)\ + Release.user, db.func.count(Release.id)) \ + .filter(Release.team == team_name) \ .group_by(Release.team) return query @@ -289,8 +318,8 @@ def package_info(package_name): :return: """ query = db.session.query( - Package.name, db.func.count(Package.id))\ - .filter(Package.name == package_name)\ + Package.name, db.func.count(Package.id)) \ + .filter(Package.name == package_name) \ .group_by(Package.name) return query @@ -324,10 +353,10 @@ def package_versions(platform=None): Package.name, db.func.max(Package.stime).label('max_stime')) \ .filter(Package.status == 'SUCCESSFUL') if platform: # filter releases not on this platform - sub_q = sub_q\ - .join(Release)\ + sub_q = sub_q \ + .join(Release) \ .filter(Release.platforms.any(Platform.name == platform)) - sub_q = sub_q\ + sub_q = sub_q \ .group_by(Package.name) \ .subquery() @@ -391,19 +420,7 @@ def count_releases(user=None, package=None, team=None, platform=None, status=Non query = query.filter(Release.stime <= ftime) if rollback is not None: - if rollback is True: - # Only count releases which have a rollback package - query = query.filter( - Release.packages.any(Package.rollback == True) - ) - elif rollback is False: - # Only count releases which do not have any rollback packages - query = query.filter( - ~Release.packages.any(Package.rollback == True) - ) - else: # What the hell did you pass? - raise TypeError("Bad rollback parameter: '{}', type {}. Boolean expected.".format( - rollback, type(rollback))) + query = _filter_release_rollback(query, rollback) if status: query = _filter_release_status(query, status) @@ -464,7 +481,7 @@ def platform_info(platform_name): query = db.session.query( Platform.name, db.func.count(Platform.id)) \ - .filter(Platform.name == platform_name)\ + .filter(Platform.name == platform_name) \ .group_by(Platform.name) return query @@ -483,3 +500,116 @@ def platform_list(): return query +def stats_release_time(unit, summarize_by_unit=False, **kwargs): + """ + Return stats by time from the given arguments + + Functions in this file usually return a query object, but here we are + returning the result, as there are several queries in play. + + :param summarize_by_unit: Passed to add_release_by_time_to_dict() + :param unit: Passed to add_release_by_time_to_dict() + """ + + root_query = db.session.query(Release.id, Release.stime).join(Package) + root_query = apply_filters(root_query, kwargs) + + # Build queries for the individual stats + q_normal_successful = _filter_release_status( + _filter_release_rollback(root_query, rollback=False), 'SUCCESSFUL' + ) + q_normal_failed = _filter_release_status( + _filter_release_rollback(root_query, rollback=False), 'FAILED' + ) + q_rollback_successful = _filter_release_status( + _filter_release_rollback(root_query, rollback=True), 'SUCCESSFUL' + ) + q_rollback_failed = _filter_release_status( + _filter_release_rollback(root_query, rollback=True), 'FAILED' + ) + + output_dict = OrderedDict() + + add_releases_by_time_to_dict( + q_normal_successful, output_dict, ('normal', 'successful'), unit, summarize_by_unit) + add_releases_by_time_to_dict( + q_normal_failed, output_dict, ('normal', 'failed'), unit, summarize_by_unit) + add_releases_by_time_to_dict( + q_rollback_successful, output_dict, ('rollback', 'successful'), unit, + summarize_by_unit) + add_releases_by_time_to_dict( + q_rollback_failed, output_dict, ('rollback', 'failed'), unit, summarize_by_unit) + + return output_dict + + +def add_releases_by_time_to_dict(query, releases_dict, t_category, unit='month', + summarize_by_unit=False): + """ + Take a query and add each of its releases to a dictionary, broken down by time + + :param dict releases_dict: Dict to add to + :param tuple t_category: tuple of headings, i.e. (, ) + :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 + :return: + """ + + for release in query: + if summarize_by_unit: + tree_args = [str(getattr(release.stime, unit))] + else: + if unit == 'year': + tree_args = [str(release.stime.year)] + elif unit == 'month': + tree_args = [str(release.stime.year), str(release.stime.month)] + elif unit == 'week': + # First two args of isocalendar(), year and week + tree_args = [str(i) for i in release.stime.isocalendar()][0:2] + elif unit == 'iso': + tree_args = [str(i) for i in release.stime.isocalendar()] + elif unit == 'day': + tree_args = [str(release.stime.year), str(release.stime.month), + str(release.stime.day)] + elif unit == 'hour': + tree_args = [str(release.stime.year), str(release.stime.month), + str(release.stime.day), str(release.stime.hour)] + else: + raise InvalidUsage( + 'Invalid unit "{}" specified for release breakdown'.format( + unit)) + # Append categories + print(tree_args) + tree_args += t_category + append_tree_recursive(releases_dict, tree_args[0], tree_args) + + +def append_tree_recursive(tree, parent, nodes): + """ + Recursively place the nodes under each other + + :param dict tree: The dictionary we are operating on + :param parent: The parent for this node + :param nodes: The list of nodes + :return: + """ + print('Called recursive function with args:\n{}, {}, {}'.format( + str(tree), str(parent), str(nodes))) + try: + # Get the child, one after the parent + child = nodes[nodes.index(parent) + 1] + except IndexError: + # Must be at end + if parent in tree: + tree[parent] += 1 + else: + tree[parent] = 1 + return tree + + # Otherwise recurse again + if parent not in tree: + tree[parent] = {} + # Child becomes the parent + append_tree_recursive(tree[parent], child, nodes) diff --git a/orlo/route_stats.py b/orlo/route_stats.py index f25e06a..329869c 100644 --- a/orlo/route_stats.py +++ b/orlo/route_stats.py @@ -299,3 +299,32 @@ def stats_package(package=None): package_stats = build_stats_dict('package', package_list, stime=stime, ftime=ftime) return jsonify(package_stats) + + +@app.route('/stats/by_date') +def stats_by_date(): + """ + Return release release_stats by date + + :query string unit: Unit to group by, i.e. year, month, week, day, hour + :query boolean summarize_by_unit: Don't build hierarchy, just summarize by the unit + :return: + + This endpoint also allows filtering on the same fields as GET /releases, e.g stime_gt. See + that endpoint for documentation. + """ + + # Get the filters into an args directory, if they are set + filters = dict((k, v) for k, v in request.args.items()) + unit = filters.pop('unit', 'month') + + summarize_by_unit = False + if filters.pop('summarize_by_unit', False): + summarize_by_unit = True + + # Returns releases and their time by rollback and status + release_stats = queries.stats_release_time(unit, summarize_by_unit, **filters) + + return jsonify(release_stats) + + diff --git a/setup.py b/setup.py index b586103..5de869f 100755 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ import multiprocessing # nopep8 -VERSION = '0.0.5-14' +VERSION = '0.0.5-19' version_file = open('./orlo/_version.py', 'w') version_file.write("__version__ = '{}'".format(VERSION)) version_file.close() diff --git a/tests/test_queries.py b/tests/test_queries.py index 1ff73ed..f3de4eb 100644 --- a/tests/test_queries.py +++ b/tests/test_queries.py @@ -662,7 +662,7 @@ def test_releases_with_bad_limit(self): Test releases raises InvalidUsage when limit is not an int """ args = { - 'limit': None, + 'limit': 'bad_limit', } with self.assertRaises(orlo.exceptions.InvalidUsage): orlo.queries.releases(**args) @@ -672,7 +672,110 @@ def test_releases_with_bad_offset(self): Test releases raises InvalidUsage when offset is not an int """ args = { - 'offset': None, + 'offset': 'bad_offset', } with self.assertRaises(orlo.exceptions.InvalidUsage): orlo.queries.releases(**args) + + +class ReleaseTimeTest(OrloQueryTest): + """ + Test queries.stats_release_time + """ + ARGS = { + 'stime_gt': arrow.utcnow().replace(hours=-1), + 'stime_lt': arrow.utcnow().replace(hours=+1) + } + + def setUp(self): + super(OrloQueryTest, self).setUp() + + for r in range(0, 7): + self._create_finished_release() + + def test_append_tree_recursive(self): + """ + Test that append_tree_recursive returns a properly structured dictionary + """ + tree = {} + nodes = ['apple', 'orange'] + orlo.queries.append_tree_recursive(tree, nodes[0], nodes) + self.assertEqual(tree, {'apple': {'orange': 1}}) + + def test_append_tree_recursive_adds(self): + """ + Test that append_tree_recursive correctly adds one when called on the same path + """ + tree = {} + nodes = ['apple', 'orange'] + orlo.queries.append_tree_recursive(tree, nodes[0], nodes) + orlo.queries.append_tree_recursive(tree, nodes[0], nodes) + self.assertEqual(tree, {'apple': {'orange': 2}}) + + def test_release_time_month(self): + """ + Test queries.add_releases_by_time_to_dict by month + """ + result = orlo.queries.stats_release_time('month', **self.ARGS) + year = str(arrow.utcnow().year) + month = str(arrow.utcnow().month) + self.assertEqual(7, result[year][month]['normal']['successful']) + + def test_release_time_week(self): + """ + Test queries.add_releases_by_time_to_dict by week + """ + result = orlo.queries.stats_release_time('week', **self.ARGS) + year, week, day = arrow.utcnow().isocalendar() + self.assertEqual(7, result[str(year)][str(week)]['normal']['successful']) + + def test_release_time_year(self): + """ + Test queries.add_releases_by_time_to_dict by year + """ + result = orlo.queries.stats_release_time('year', **self.ARGS) + year = str(arrow.utcnow().year) + self.assertEqual(7, result[str(year)]['normal']['successful']) + + def test_release_time_day(self): + """ + Test queries.add_releases_by_time_to_dict by day + """ + result = orlo.queries.stats_release_time('day', **self.ARGS) + year = str(arrow.utcnow().year) + month = str(arrow.utcnow().month) + day = str(arrow.utcnow().day) + self.assertEqual( + 7, + result[year][month][day]['normal']['successful'], + ) + + def test_release_time_hour(self): + """ + Test queries.add_releases_by_time_to_dict by hour + """ + result = orlo.queries.stats_release_time('hour', **self.ARGS) + year = str(arrow.utcnow().year) + month = str(arrow.utcnow().month) + day = str(arrow.utcnow().day) + hour = str(arrow.utcnow().hour) + self.assertEqual( + 7, + result[year][month][day][hour]['normal']['successful'], + ) + + def test_release_time_with_only_this_unit(self): + """ + Test queries.add_releases_by_time_to_dict with only_this_unit + + Should break down by only the unit given + """ + result = orlo.queries.stats_release_time('hour', summarize_by_unit=True, **self.ARGS) + hour = str(arrow.utcnow().hour) + self.assertEqual( + 7, + result[hour]['normal']['successful'], + ) + + def test_release_time_with_unit_day(self): + pass \ No newline at end of file diff --git a/tests/test_route_stats.py b/tests/test_route_stats.py index 964b002..1ee54ec 100644 --- a/tests/test_route_stats.py +++ b/tests/test_route_stats.py @@ -1,5 +1,6 @@ from __future__ import print_function from tests.test_orm import OrloDbTest +import arrow import unittest __author__ = 'alforbes' @@ -127,3 +128,51 @@ def test_stats_package_returns_dict_with_package(self): response = self.client.get(self.ENDPOINT + '/test-package') self.assertIsInstance(response.json, dict) + +class TimeBasedStatsTest(StatsTest): + ENDPOINT = '/stats/by_date' + + def test_result_includes_normals(self): + unittest.skip("Not suitable test for this endpoint") + + def test_result_includes_rollbacks(self): + unittest.skip("Not suitable test for this endpoint") + + def test_result_includes_totals(self): + unittest.skip("Not suitable test for this endpoint") + + def test_with_invalid_stime(self): + # TODO the stats endpoints should be made consistent with the others by calling + # apply_filters on the query parameters + unittest.skip("Not suitable test for this endpoint") + + def test_stats_by_date_with_year(self): + """ + Test /stats/by_date/year + """ + year = arrow.utcnow().year + response = self.client.get(self.ENDPOINT + '?stime_gt={}-01-01'.format(year)) + self.assert200(response) + + def test_stats_by_date_with_year_month(self): + """ + Test /stats/by_date/year + """ + year = arrow.utcnow().year + month = arrow.utcnow().month + response = self.client.get(self.ENDPOINT + '?stime_gt={}-{}-01'.format(year, month)) + self.assert200(response) + + def test_stats_by_date_with_unit_day(self): + """ + Test /stats/by_date/year + """ + response = self.client.get(self.ENDPOINT + '?unit=day') + self.assert200(response) + + def test_stats_by_date_with_summarize_by_unit_day(self): + """ + Test /stats/by_date/year + """ + response = self.client.get(self.ENDPOINT + '?unit=day&summarize_by_unit=1') + self.assert200(response) From f39205020beac765761c432dcdfe42e14e9b1501 Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Mon, 18 Jan 2016 15:08:26 +0000 Subject: [PATCH 14/18] Move orlo.conf to orlo/orlo.ini --- debian/dirs | 1 + debian/install | 1 + docs/install.rst | 4 ++-- etc/orlo.ini | 11 +++++++++++ orlo/cli.py | 4 ++-- orlo/config.py | 2 +- orlo/route_api.py | 2 +- 7 files changed, 19 insertions(+), 6 deletions(-) create mode 100644 debian/dirs create mode 100644 etc/orlo.ini diff --git a/debian/dirs b/debian/dirs new file mode 100644 index 0000000..cd0c3df --- /dev/null +++ b/debian/dirs @@ -0,0 +1 @@ +/var/log/orlo diff --git a/debian/install b/debian/install index 1b5f716..a7f6de6 100644 --- a/debian/install +++ b/debian/install @@ -1 +1,2 @@ bin/orlo usr/bin +etc/orlo.ini /etc/orlo diff --git a/docs/install.rst b/docs/install.rst index ff76ab6..eeade3f 100644 --- a/docs/install.rst +++ b/docs/install.rst @@ -80,7 +80,7 @@ From the psql prompt: CREATE DATABASE postgres=# -Update uri under [db] in orlo.conf, e.g. +Update uri under [db] in orlo.ini, e.g. :: @@ -96,7 +96,7 @@ Create a directory for the db, e.g. : mkdir /var/lib/orlo chown orlo:root /var/lib/orlo -Update the uri under [db] in orlo.conf to point to a file in this directory: +Update the uri under [db] in orlo.ini to point to a file in this directory: :: diff --git a/etc/orlo.ini b/etc/orlo.ini new file mode 100644 index 0000000..d4d7d00 --- /dev/null +++ b/etc/orlo.ini @@ -0,0 +1,11 @@ +[db] +uri = sqlite:// + +[main] +time_zone = UTC +propagate_exceptions = true +time_format = %Y-%m-%dT%H:%M:%SZ + +[logging] +file = /var/log/orlo/app.log +debug = false diff --git a/orlo/cli.py b/orlo/cli.py index b2d8259..ebe532e 100644 --- a/orlo/cli.py +++ b/orlo/cli.py @@ -15,7 +15,7 @@ def parse_args(): p_config = argparse.ArgumentParser(add_help=False) p_config.add_argument('--file', '-f', dest='filepath', help="File to write to", - default='/etc/orlo.conf') + default='/etc/orlo/orlo.ini') p_database = argparse.ArgumentParser(add_help=False) p_server = argparse.ArgumentParser(add_help=False) @@ -54,7 +54,7 @@ def setup_database(args): if config.get('db', 'uri') == 'sqlite://': print("Warning: setting up in-memory database, this is " "probably not what you want!\n" - "Please configure db:uri in /etc/orlo.conf") + "Please configure db:uri in /etc/orlo/orlo.ini") db.create_all() diff --git a/orlo/config.py b/orlo/config.py index a8e3dac..59ed67c 100644 --- a/orlo/config.py +++ b/orlo/config.py @@ -18,4 +18,4 @@ config.set('logging', 'debug', 'false') config.set('logging', 'file', 'disabled') -config.read('/etc/orlo.conf') +config.read('/etc/orlo/orlo.ini') diff --git a/orlo/route_api.py b/orlo/route_api.py index 6ab39b0..fe2942f 100644 --- a/orlo/route_api.py +++ b/orlo/route_api.py @@ -261,7 +261,7 @@ def get_releases(release_id=None): "NOT_STARTED", "IN_PROGRESS", "SUCCESSFUL", "FAILED" **Note for time arguments**: - The timestamp format you must use is specified in /etc/orlo.conf. All times are UTC. + The timestamp format you must use is specified in /etc/orlo/orlo.ini. All times are UTC. **Note on status**: The release status is calculated from the packages it contains. The possible values are From 056d3940456de6d0fffbc8d4bd777b81809dd7b2 Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Mon, 18 Jan 2016 15:14:47 +0000 Subject: [PATCH 15/18] Rename package from python-orlo to orlo Not really a python library package! --- bin/orlo | 2 +- debian/control | 4 ++-- etc/orlo.service | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/orlo b/bin/orlo index bd89ef1..3376fab 100755 --- a/bin/orlo +++ b/bin/orlo @@ -1,5 +1,5 @@ #!/bin/bash -/usr/share/python/python-orlo/bin/python /usr/share/python/python-orlo/lib/python2.7/site-packages/orlo/cli.py $@ +/usr/share/python/orlo/bin/python /usr/share/python/orlo/lib/python2.7/site-packages/orlo/cli.py $@ diff --git a/debian/control b/debian/control index ea197ef..3e8dd0a 100644 --- a/debian/control +++ b/debian/control @@ -1,11 +1,11 @@ -Source: python-orlo +Source: orlo Section: python Priority: extra Maintainer: Alex Forbes Build-Depends: debhelper (>= 9), python, dh-virtualenv, gcc, python-dev, postgresql-server-dev-all Standards-Version: 3.9.5 -Package: python-orlo +Package: orlo Architecture: all Depends: ${python:Depends}, ${misc:Depends} Description: An API for tracking deployments, written with Python, Flask and SqlAlchemy. diff --git a/etc/orlo.service b/etc/orlo.service index bd88614..91fa7e1 100644 --- a/etc/orlo.service +++ b/etc/orlo.service @@ -3,13 +3,13 @@ [Unit] Description=orlo After=network.target -ConditionPathExists=/usr/share/python/python-orlo/bin/gunicorn +ConditionPathExists=/usr/share/python/orlo/bin/gunicorn [Service] Type=simple User=orlo Group=orlo -ExecStart=/usr/share/python/python-orlo/bin/gunicorn -w 4 -b 127.0.0.1:8080 orlo:app +ExecStart=/usr/share/python/orlo/bin/gunicorn -w 4 -b 127.0.0.1:8080 orlo:app [Install] WantedBy=multi-user.target From 2f300c900e630060efeac4fbf0ad1a8abbabf43b Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Mon, 18 Jan 2016 16:45:39 +0000 Subject: [PATCH 16/18] Deb packaging fixes * Move systemd unit * Add dh_systemd to rules * set -e in preinst * Add --no-test to dh_virtualenv (already have CI) --- Vagrantfile | 2 +- debian/control | 2 +- debian/install | 1 + debian/preinst | 8 +++++--- debian/rules | 5 ++++- {etc => systemd}/orlo.service | 0 6 files changed, 12 insertions(+), 6 deletions(-) rename {etc => systemd}/orlo.service (100%) diff --git a/Vagrantfile b/Vagrantfile index 4443bba..a7f3b7c 100644 --- a/Vagrantfile +++ b/Vagrantfile @@ -76,7 +76,7 @@ Vagrant.configure(2) do |config| | sudo -u postgres -i psql # Build tools - sudo apt-get -y install build-essential git-buildpackage debhelper python-dev + sudo apt-get -y install build-essential git-buildpackage debhelper python-dev dh-systemd sudo pip install --upgrade pip sudo pip install sphinx sphinxcontrib-httpdomain diff --git a/debian/control b/debian/control index 3e8dd0a..b1dac0a 100644 --- a/debian/control +++ b/debian/control @@ -2,7 +2,7 @@ Source: orlo Section: python Priority: extra Maintainer: Alex Forbes -Build-Depends: debhelper (>= 9), python, dh-virtualenv, gcc, python-dev, postgresql-server-dev-all +Build-Depends: debhelper (>= 9), python, dh-virtualenv, gcc, python-dev, postgresql-server-dev-all, dh-systemd Standards-Version: 3.9.5 Package: orlo diff --git a/debian/install b/debian/install index a7f6de6..f52720d 100644 --- a/debian/install +++ b/debian/install @@ -1,2 +1,3 @@ bin/orlo usr/bin etc/orlo.ini /etc/orlo +systemd/orlo.service /lib/systemd/system diff --git a/debian/preinst b/debian/preinst index 65268e7..fb1edca 100644 --- a/debian/preinst +++ b/debian/preinst @@ -1,11 +1,13 @@ #!/bin/bash +set -ue + if grep -q "^orlo" /etc/passwd; then : else useradd orlo -s /bin/false -U fi -mkdir -p /var/lib/orlo -chown orlo:orlo /var/lib/orlo -chmod 755 /var/lib/orlo +mkdir -p /var/{lib,log}/orlo +chown orlo:orlo /var/{lib,log}/orlo +chmod 755 /var/{lib,log}/orlo diff --git a/debian/rules b/debian/rules index 299e309..1d28e71 100755 --- a/debian/rules +++ b/debian/rules @@ -1,4 +1,7 @@ #!/usr/bin/make -f %: - dh $@ --with python-virtualenv + dh $@ --with systemd --with python-virtualenv + +override_dh_virtualenv: + dh_virtualenv --no-test diff --git a/etc/orlo.service b/systemd/orlo.service similarity index 100% rename from etc/orlo.service rename to systemd/orlo.service From a920b980414950817d54e12b5cf8ed49a04d7efc Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Tue, 19 Jan 2016 11:50:39 +0000 Subject: [PATCH 17/18] Add tests for stop package --- tests/test_contract.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_contract.py b/tests/test_contract.py index d3eda7a..a12dd6a 100644 --- a/tests/test_contract.py +++ b/tests/test_contract.py @@ -282,6 +282,32 @@ def test_references_json_conversion(self): doc = json.loads(release.references) self.assertIsInstance(doc, list) + def test_stop_package_success_true(self): + """ + Test stop_package when success is false + """ + release_id = self._create_release() + package_id = self._create_package(release_id) + self._start_package(release_id, package_id) + self._stop_package(release_id, package_id, success=True) + + release = db.session.query(Release).filter(Release.id == release_id).one() + package = release.packages[0] + self.assertEqual(package.status, 'SUCCESSFUL') + + def test_stop_package_success_false(self): + """ + Test stop_package when success is false + """ + release_id = self._create_release() + package_id = self._create_package(release_id) + self._start_package(release_id, package_id) + self._stop_package(release_id, package_id, success=False) + + release = db.session.query(Release).filter(Release.id == release_id).one() + package = release.packages[0] + self.assertEqual(package.status, 'FAILED') + class GetContractTest(OrloHttpTest): """ From ce99762f9d6ceef168c24b2cdfac210830fba00a Mon Sep 17 00:00:00 2001 From: Alex Forbes Date: Tue, 19 Jan 2016 11:53:05 +0000 Subject: [PATCH 18/18] Bump version to 0.1.0 --- setup.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.py b/setup.py index 5de869f..6ef5264 100755 --- a/setup.py +++ b/setup.py @@ -5,7 +5,7 @@ import multiprocessing # nopep8 -VERSION = '0.0.5-19' +VERSION = '0.1.0' version_file = open('./orlo/_version.py', 'w') version_file.write("__version__ = '{}'".format(VERSION)) version_file.close()