← Back to team overview

openlp-dev team mailing list archive

Re: [Merge] lp:~openlp-dev/openlp/apifixes into lp:openlp

 

Good progress with some inline comments.
In addition we need loggining (debug) on all the api's so we know  what has been called.  This way our log file will have the picture of what is going on.
Can we have constants for magic numbers  return '', 200 should be somthing like return '', http.success.  

Diff comments:

> 
> === modified file 'openlp/core/api/http/server.py'
> --- openlp/core/api/http/server.py	2018-01-07 04:36:45 +0000
> +++ openlp/core/api/http/server.py	2018-09-03 21:33:52 +0000
> @@ -46,6 +40,9 @@
>  from openlp.core.common.settings import Settings
>  from openlp.core.threading import ThreadWorker, run_thread
>  
> +

No need for blank lines here - may be other places

> +from openlp.core.api.ng import app as application
> +
>  log = logging.getLogger(__name__)
>  
>  
> 
> === added directory 'openlp/core/api/ng'
> === added file 'openlp/core/api/ng/__init__.py'
> --- openlp/core/api/ng/__init__.py	1970-01-01 00:00:00 +0000
> +++ openlp/core/api/ng/__init__.py	2018-09-03 21:33:52 +0000
> @@ -0,0 +1,16 @@
> +from flask import Flask, jsonify
> +from waitress import serve
> +
> +from openlp.core.api.ng.endpoints.old import old_register_blueprints
> +from openlp.core.api.ng.endpoints.v1 import v1_register_blueprints
> +from openlp.core.api.ng.main import main_views
> +from openlp.core.common.applocation import AppLocation
> +
> +app = Flask(__name__)
> +
> +app.register_blueprint(main_views)
> +old_register_blueprints(app)
> +v1_register_blueprints(app)

These should be v3 as the api is on its third release and will match the existing deployed code and versioning.

> +
> +def register_blueprint(blueprint, url_prefix=None):
> +    app.register_blueprint(blueprint, url_prefix)
> 
> === added file 'openlp/core/api/ng/lib.py'
> --- openlp/core/api/ng/lib.py	1970-01-01 00:00:00 +0000
> +++ openlp/core/api/ng/lib.py	2018-09-03 21:33:52 +0000
> @@ -0,0 +1,29 @@
> +import json
> +from flask import jsonify
> +from functools import wraps
> +
> +def requires_login(f):
> +    @wraps(f)
> +    def decorated(*args, **kwargs):
> +        return f(*args, **kwargs)
> +    return decorated
> +
> +
> +def login_required(f):
> +    @wraps(f)
> +    def decorated(*args, **kwargs):
> +        return f(*args, **kwargs)

is this missing some code?

> +    return decorated
> +
> +
> +def old_success_response():
> +    return jsonify({'results': {'sucess': True}})
> +
> +
> +def extract_request(json_string, name):
> +    try:
> +        if json_string:
> +            return json.loads(json_string)['request'][name]
> +    except KeyError:
> +        pass
> +    return None
> \ No newline at end of file
> 
> === added file 'openlp/core/ui/media/remote.py'
> --- openlp/core/ui/media/remote.py	1970-01-01 00:00:00 +0000
> +++ openlp/core/ui/media/remote.py	2018-09-03 21:33:52 +0000
> @@ -0,0 +1,98 @@
> +# -*- coding: utf-8 -*-
> +# vim: autoindent shiftwidth=4 expandtab textwidth=120 tabstop=4 softtabstop=4
> +
> +###############################################################################
> +# OpenLP - Open Source Lyrics Projection                                      #
> +# --------------------------------------------------------------------------- #
> +# Copyright (c) 2008-2018 OpenLP Developers                                   #
> +# --------------------------------------------------------------------------- #
> +# This program is free software; you can redistribute it and/or modify it     #
> +# under the terms of the GNU General Public License as published by the Free  #
> +# Software Foundation; version 2 of the License.                              #
> +#                                                                             #
> +# This program is distributed in the hope that it will be useful, but WITHOUT #
> +# ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or       #
> +# FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for    #
> +# more details.                                                               #
> +#                                                                             #
> +# You should have received a copy of the GNU General Public License along     #
> +# with this program; if not, write to the Free Software Foundation, Inc., 59  #
> +# Temple Place, Suite 330, Boston, MA 02111-1307 USA                          #
> +###############################################################################
> +"""
> +The :mod:`~openlp.core.api.endpoint` module contains various API endpoints
> +"""
> +import logging
> +from flask import jsonify, Blueprint
> +
> +from openlp.core.api.ng import app
> +from openlp.core.api.ng.lib import login_required
> +from openlp.core.common.registry import Registry
> +
> +log = logging.getLogger(__name__)
> +
> +v1_media = Blueprint('v1-media-controller', __name__)
> +old_media = Blueprint('old-media-controller', __name__)
> +
> +
> +@v1_media.route('/play')
> +@login_required
> +def media_play(request):
> +    media = Registry().get('media_controller')
> +    live = Registry().get('live_controller')
> +    status = media.media_play(live, False)
> +    if status:
> +        return '', 204
> +    return 'Not sure what to send here...'
> +    return {'success': status}
> +
> +
> +@v1_media.route('/pause')
> +@login_required
> +def media_pause(request):
> +    media = Registry().get('media_controller')
> +    live = Registry().get('live_controller')
> +    status = media.media_pause(live)
> +    if status:
> +        return '', 204
> +    return 'Not sure what to send here...'
> +
> +
> +@v1_media.route('/stop')
> +@login_required
> +def media_stop(request):
> +    Registry().get('live_controller').mediacontroller_live_stop.emit()
> +    return '', 204
> +
> +
> +# -------------- DEPRECATED ------------------------
> +
> +@old_media.route('/play')
> +@login_required
> +def old_media_play(request):
> +    media = Registry().get('media_controller')
> +    live = Registry().get('live_controller')
> +    status = media.media_play(live, False)
> +    return jsonify({'success': status})
> +
> +
> +@old_media.route('/pause')
> +@login_required
> +def old_media_pause(request):
> +    media = Registry().get('media_controller')
> +    live = Registry().get('live_controller')
> +    status = media.media_pause(live)
> +    return jsonify({'success': status})
> +
> +
> +@old_media.route('/stop')
> +@login_required
> +def old_media_stop(request):
> +    Registry().get('live_controller').mediacontroller_live_stop.emit()
> +    return ''
> +
> +# -------------- END OF DEPRECATED ------------------------
> +
> +def register_views():
> +    app.register_blueprint(v1_media, url_prefix='/v1/api/media/')

we wanted to loose /api/ as this is an api and it is obvious.  More likely in many places as well.

> +    app.register_blueprint(old_media, url_prefix='/api/media/')
> \ No newline at end of file


-- 
https://code.launchpad.net/~openlp-dev/openlp/apifixes/+merge/354205
Your team OpenLP Development is subscribed to branch lp:~openlp-dev/openlp/apifixes.


Follow ups