refactor server.py for using Blueprint#3033
refactor server.py for using Blueprint#3033JanEisermann wants to merge 25 commits intoOpen-MSS:developfrom
Conversation
mslib/mswms/app/__init__.py
Outdated
| SCRIPT_NAME = os.environ.get('SCRIPT_NAME', '/') | ||
|
|
||
| docs_bp = Blueprint( | ||
| "docs", |
There was a problem hiding this comment.
capital letters, see https://peps.python.org/pep-0008/#constants
mslib/mscolab/app/__init__.py
Outdated
| # This can be used to set a location by SCRIPT_NAME for testing. e.g. export SCRIPT_NAME=/demo/ | ||
| SCRIPT_NAME = os.environ.get('SCRIPT_NAME', '/') | ||
|
|
||
| docs_bp = Blueprint( |
There was a problem hiding this comment.
capital letters, see https://peps.python.org/pep-0008/#constants
mslib/mscolab/app/__init__.py
Outdated
| @@ -57,21 +57,24 @@ def file_exists(filepath=None): | |||
| DOCS_STATIC_DIR = os.path.join(DOCS_SERVER_PATH, 'static') | |||
There was a problem hiding this comment.
move the section before the first function
mslib/mswms/app/__init__.py
Outdated
| logging.warning(message) | ||
|
|
||
|
|
||
| def file_exists(filepath=None): |
There was a problem hiding this comment.
may be when it is duplicated move it to mslib.utils and import it?
mslib/mswms/app/__init__.py
Outdated
| return False | ||
|
|
||
|
|
||
| DOCS_SERVER_PATH = os.path.dirname(os.path.abspath(mslib.__file__)) |
There was a problem hiding this comment.
move the constants to the beginning after the import block, see https://peps.python.org/pep-0008/#constants
mslib/mscolab/server.py
Outdated
| @@ -416,7 +416,7 @@ def user_register_handler(): | |||
| if APP.config['MAIL_ENABLED']: | |||
There was a problem hiding this comment.
have also a look if we have to use current_app instead of the imported APP
mslib/mscolab/app/__init__.py
Outdated
| # This can be used to set a location by SCRIPT_NAME for testing. e.g. export SCRIPT_NAME=/demo/ | ||
| SCRIPT_NAME = os.environ.get('SCRIPT_NAME', '/') | ||
|
|
||
| DOCS_BP = Blueprint( |
There was a problem hiding this comment.
could that also be moved to a blueprints/docs?
| CHAT_BP = Blueprint('chat', __name__) | ||
|
|
||
|
|
||
| @CHAT_BP.route('/undo_changes', methods=["POST"]) |
There was a problem hiding this comment.
this belongs to operation
when the filemanager "fm" is involed this goes to operation
the chatmanager "cm" makes it to chat blueprint
|
|
||
|
|
||
| @CHAT_BP.route('/get_all_changes', methods=['GET']) | ||
| @verify_user |
There was a problem hiding this comment.
this belongs to operation
|
|
||
| @CHAT_BP.route('/get_change_content', methods=['GET']) | ||
| @verify_user | ||
| def get_change_content(): |
There was a problem hiding this comment.
this belongs to operation
|
|
||
| @CHAT_BP.route('/set_version_name', methods=['POST']) | ||
| @verify_user | ||
| def set_version_name(): |
There was a problem hiding this comment.
this belongs to operation
|
|
||
| @OPERATION_BP.route('/active_users', methods=["GET"]) | ||
| @verify_user | ||
| def active_users(): |
There was a problem hiding this comment.
this maybe can go with status to an info blueprint?
| from mslib.mscolab.app import APP | ||
| from mslib.utils import conditional_decorator | ||
|
|
||
|
|
There was a problem hiding this comment.
The not route related code should be on an other place
check_login, register_user ... verify_user these should be not in the blueprint file
Please look if they can be moved to mslib.utils.auth.py and imported where needed
| return wrapper | ||
|
|
||
|
|
||
| AUTH_BP = Blueprint('auth', __name__) |
There was a problem hiding this comment.
from here that is the part of the blueprint
mslib/mswms/app/__init__.py
Outdated
| # This can be used to set a location by SCRIPT_NAME for testing. e.g. export SCRIPT_NAME=/demo/ | ||
| SCRIPT_NAME = os.environ.get('SCRIPT_NAME', '/') | ||
|
|
||
| DOCS_BP = Blueprint( |
There was a problem hiding this comment.
this should become also an own blueprints docs.py
mslib/utils/file_exists.py
Outdated
| mslib.utils.file_exists | ||
| ~~~~~~~~~~~~~~~~~ | ||
| function for app |
There was a problem hiding this comment.
verifies a given filepath is a file
ReimarBauer
left a comment
ReimarBauer
left a comment
There was a problem hiding this comment.
That's good progress. We can already see it's becoming much more readable. You need to rearrange a few things.
The blueprints should contain as few additional functions as possible.
Especially not ones that need to be imported from other blueprints.
see comments
JanEisermann
force-pushed
the
inkludingBP
branch
from
efd14d0 to
570f809
Compare
| (url_for('help'), 'Help'), | ||
| (url_for('docs.index'), 'Mission Support System', | ||
| ((url_for('docs.about'), 'About'), | ||
| (url_for('docs.install'), 'Install'), |
There was a problem hiding this comment.
(url_for("gallery.plots"), 'Gallery'), is missing
| from mslib.utils.get_content import get_content | ||
|
|
||
| GALLERY_BP = Blueprint('gallery', __name__, template_folder='templates') | ||
|
|
There was a problem hiding this comment.
the gallery feature requests data on /static and the files are on the STATIC_LOCATION
you need to add static_url_path and static_folder
|
|
||
| @AUTH_BP.route('/reset_password/<token>', methods=['GET', 'POST']) | ||
| def reset_password(token): | ||
| try: |
There was a problem hiding this comment.
reset_password wants to access user/status_password.html
this gives currently jinja2.exceptions.TemplateNotFound: user/status_password.html
| fm.modify_user(user, attribute="confirmed_on", value=datetime.datetime.now(tz=datetime.timezone.utc)) | ||
| fm.modify_user(user, attribute="confirmed", value=True) | ||
| return render_template('user/confirmed.html', username=user.username) | ||
|
|
There was a problem hiding this comment.
else:
logging.warning("To send emails, the value of MAIL_ENABLED in conf.py should be set to True.")
return render_template('errors/403.html'), 403
this is missing for the developer case
|
|
||
| @AUTH_BP.route("/reset_request", methods=['GET', 'POST']) | ||
| def reset_request(): | ||
| if APP.config['MAIL_ENABLED']: |
There was a problem hiding this comment.
when mail is not enabled it shows
jinja2.exceptions.TemplateNotFound: errors/403.html
when it is enabled it shows
jinja2.exceptions.TemplateNotFound: user/reset_request.html
| status_code = 204 | ||
| token = generate_confirmation_token(email) | ||
| confirm_url = url_for('docs.confirm_email', token=token, _external=True) | ||
| html = render_template('auth/user/activate.html', username=username, confirm_url=confirm_url) |
| try: | ||
| username = user.username | ||
| token = generate_confirmation_token(form.email.data) | ||
| reset_password_url = url_for('docs.reset_password', token=token, _external=True) |
| try: | ||
| username = user.username | ||
| token = generate_confirmation_token(form.email.data) | ||
| reset_password_url = url_for('auth.user.reset_password', token=token, _external=True) |
There was a problem hiding this comment.
it is auth.reset_password it returns an url not a path
| if APP.config['MAIL_ENABLED']: | ||
| status_code = 204 | ||
| token = generate_confirmation_token(email) | ||
| confirm_url = url_for('auth.user.confirm_email', token=token, _external=True) |
There was a problem hiding this comment.
it is auth.confirm_email it returns an url not a path
| {% if uri is defined %} | ||
| <p>Click here to | ||
| <a href=" {{ url_for(uri.path) }}">{{uri.name}}</a> | ||
| <a href=" {{ url_for(docs.uri.path) }}">{{uri.name}}</a> |
There was a problem hiding this comment.
this is likly only uri.path
I get the error message jinja2.exceptions.UndefinedError: 'docs' is undefined
| if email is False: | ||
| flash("Sorry, your token has expired or is invalid! We will need to resend your authentication email", | ||
| 'category_info') | ||
| return render_template('auth/user/status_password.html', uri={"path": "reset_request", "name": "Resend " |
| from mslib.mscolab.conf import setup_saml2_backend | ||
| from mslib.mscolab.forms import ResetPasswordForm, ResetRequestForm | ||
| from mslib.mscolab.models import User | ||
| from mslib.mscolab.app import APP |
| import werkzeug | ||
| from flask import Blueprint, request, g, jsonify, abort, send_from_directory | ||
|
|
||
| from mslib.mscolab.app import APP |
| import os | ||
| from functools import wraps | ||
|
|
||
| from flask import Blueprint, abort, send_from_directory, render_template, url_for, send_file, current_app |
There was a problem hiding this comment.
already current_app :)
in principle we stay on APP when it is initialzation-time code. All others are likly request time based. These should use current_app.
| @OPERATION_BP.route('/get_change_content', methods=['GET']) | ||
| @verify_user | ||
| def get_change_content(): | ||
| from mslib.mscolab.server import getConfig |
There was a problem hiding this comment.
is this for ommiting a circular import?
|
|
||
| from flask import Blueprint, g, request, jsonify, send_from_directory | ||
|
|
||
| from mslib.mscolab.app import APP |
| APP.register_blueprint(AUTH_BP) | ||
| APP.register_blueprint(CHAT_BP) | ||
| APP.register_blueprint(USER_BP) | ||
| APP.register_blueprint(OPERATION_BP) |
There was a problem hiding this comment.
there could be a blank line for separation
| from flask_mail import Message | ||
| from itsdangerous import URLSafeTimedSerializer, BadSignature | ||
|
|
||
| from mslib.mscolab.app import APP |
| @@ -33,10 +33,12 @@ | |||
| from PIL import Image | |||
|
|
|||
| from mslib.mscolab.app import APP | |||
2 participants
Purpose of PR?:
Fixes #2081
Does this PR introduce a breaking change?
included Blueprint, fixed test_load_no_file
If the changes in this PR are manually verified, list down the scenarios covered::
tests succeed, not manually verified
Additional information for reviewer? :
Mention if this PR is part of any design or a continuation of previous PRs
Does this PR results in some Documentation changes?
If yes, include the list of Documentation changes
Checklist:
<type>: <subject>