Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make server more flexible #576

Open
wants to merge 13 commits into
base: master
Choose a base branch
from
98 changes: 80 additions & 18 deletions gramps_webapi/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,19 @@
import sys
import time
import warnings
from threading import Thread

import click
import waitress
import webbrowser

from .api.search import get_search_indexer
from .api.util import get_db_manager, list_trees, close_db
from .app import create_app
from .auth import add_user, delete_user, fill_tree, user_db
from .const import ENV_CONFIG_FILE, TREE_MULTI
from .dbmanager import WebDbManager

logging.basicConfig()
LOG = logging.getLogger("gramps_webapi")
from .translogger import TransLogger


@click.group("cli")
Expand All @@ -58,12 +59,71 @@ def cli(ctx, config):

@cli.command("run")
@click.option("-p", "--port", help="Port to use (default: 5000)", default=5000)
@click.option("--tree", help="Tree ID", default=None)
@click.option("-t", "--tree", help="Tree ID: '*' for multi-trees", default=None)
@click.option(
"-o",
"--open",
help="Open gramps-web in browser: 'tab', 'window', or 'no'",
default="no",
)
@click.option(
"-d",
"--debug-level",
help="Debug level: 'info', 'debug', 'warning', 'critical'",
default="info",
)
@click.option("-l", "--log-file", help="Set logging file to this path", default=None)
@click.option(
"--host", help="Set the host address for server to listen on", default="127.0.0.1"
)
@click.option(
"--max-workers",
help="Maximum number of workers for frontend; requires --use-wsgi",
default=None,
)
@click.option("--use-wsgi", is_flag=True, help="Add a wsgi wrapper around server")
@click.pass_context
def run(ctx, port, tree):
def run(ctx, port, tree, host, open, debug_level, log_file, max_workers, use_wsgi):
"""Run the app."""
app = ctx.obj["app"]
app.run(port=port, threaded=True)
debug_level = debug_level.upper()
if max_workers is None:
max_workers = min(32, os.cpu_count() + 4)

def open_webbrowser():
time.sleep(1.0)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why sleep?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That gives the server time to start. If you open the page too quickly, you'll get an error.

new = {"tab": 0, "window": 1}[open]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will raise a KeyError if the user provides any unexpected string. I think it would be better to use a click choice option for the browser flag.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll take a look.

webbrowser.open("http://%s:%s" % (host, port), new=0, autoraise=True)

if open != "no":
thread = Thread(target=open_webbrowser)
thread.start()

if log_file:
file_handler = logging.FileHandler(log_file, "w+")
app.logger.addHandler(file_handler)
app.logger.setLevel(debug_level)

print("Running gramps-web...")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gramps Web API

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I debated that. On one hand, the gramps-web-api is just the REST server. Thinking of suggesting renaming gramps-web to gramps-web-frontend, plus gramps-web-api together as gramps-web (and I'm thinking about a package just called gramps-web). What do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thinking of suggesting renaming gramps-web to gramps-web-frontend, plus gramps-web-api together as gramps-web

That's exactly the naming we have. The frontend used to have a separate name (Gramps.js, "the Gramps Web frontend"), but we renamend the repository to gramps-web because it lead to confusion.

From the point of view of the combined web application, it would make sense to have a monorepo and combined packaging, but so far we still have them separated. Since the API can be used for other things as well, I think it kind of makes sense. And for proper web-based deployments, it's anyway more complicated (celery, nginx, etc.).

What we could do is to add a convenience command to download a frontend.

if open != "no":
print(f" Opening {open} on http://{host}:{port}...")

print(" Control+C to quit")
if use_wsgi:
waitress.serve(
TransLogger(
app,
setup_console_handler=False,
set_logger_level=debug_level,
),
host=host,
port=port,
threads=max_workers,
)
else:
app.run(port=port, threaded=True)
print()
print("Stopping gramps-web...")


@cli.group("user", help="Manage users.")
Expand All @@ -82,8 +142,8 @@ def user(ctx):
@click.pass_context
def user_add(ctx, name, password, fullname, email, role, tree):
"""Add a user."""
LOG.error(f"Adding user {name} ...")
app = ctx.obj["app"]
app.logger.error(f"Adding user {name} ...")
with app.app_context():
user_db.create_all()
add_user(name, password, fullname, email, role, tree)
Expand All @@ -94,8 +154,8 @@ def user_add(ctx, name, password, fullname, email, role, tree):
@click.pass_context
def user_del(ctx, name):
"""Delete a user."""
LOG.info(f"Deleting user {name} ...")
app = ctx.obj["app"]
app.logger.info(f"Deleting user {name} ...")
with app.app_context():
delete_user(name)

Expand Down Expand Up @@ -146,51 +206,55 @@ def search(ctx, tree, semantic):
ctx.obj["search_indexer"] = get_search_indexer(tree=tree, semantic=semantic)


def progress_callback_count(current: int, total: int, prev: int | None = None) -> None:
def progress_callback_count(
app, current: int, total: int, prev: int | None = None
) -> None:
if total == 0:
return
pct = int(100 * current / total)
if prev is None:
prev = current - 1
pct_prev = int(100 * prev / total)
if current == 0 or pct != pct_prev:
LOG.info(f"Progress: {pct}%")
app.logger.info(f"Progress: {pct}%")


@search.command("index-full")
@click.pass_context
def index_full(ctx):
"""Perform a full reindex."""
LOG.info("Rebuilding search index ...")
app = ctx.obj["app"]
app.logger.info("Rebuilding search index ...")
db_manager = ctx.obj["db_manager"]
indexer = ctx.obj["search_indexer"]
db = db_manager.get_db().db

t0 = time.time()
try:
indexer.reindex_full(db, progress_cb=progress_callback_count)
indexer.reindex_full(app, db, progress_cb=progress_callback_count)
except:
LOG.exception("Error during indexing")
app.logger.exception("Error during indexing")
finally:
close_db(db)
LOG.info(f"Done building search index in {time.time() - t0:.0f} seconds.")
app.logger.info(f"Done building search index in {time.time() - t0:.0f} seconds.")


@search.command("index-incremental")
@click.pass_context
def index_incremental(ctx):
"""Perform an incremental reindex."""
app = ctx.obj["app"]
db_manager = ctx.obj["db_manager"]
indexer = ctx.obj["search_indexer"]
db = db_manager.get_db().db

try:
indexer.reindex_incremental(db, progress_cb=progress_callback_count)
except Exception:
LOG.exception("Error during indexing")
app.logger.exception("Error during indexing")
finally:
close_db(db)
LOG.info("Done updating search index.")
app.logger.info("Done updating search index.")


@cli.group("tree", help="Manage trees.")
Expand Down Expand Up @@ -236,8 +300,6 @@ def migrate_gramps_db(ctx):


if __name__ == "__main__":
LOG.setLevel(logging.INFO)

cli(
prog_name="python3 -m gramps_webapi"
) # pylint:disable=no-value-for-parameter,unexpected-keyword-arg
137 changes: 137 additions & 0 deletions gramps_webapi/translogger.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
# -*- coding: utf-8 -*-
# Copyright (c) 2005 Ian Bicking and contributors; written for Paste (http://pythonpaste.org)
# Licensed under the MIT license: http://www.opensource.org/licenses/mit-license.php
"""
Middleware for logging requests, using Apache combined log format
"""

import logging
import time
import urllib.parse


class TransLogger(object):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(object) is not needed (I know this is copy-pasted from somewhere else, but we can modernize a bit)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can fix a couple of other things there too. Now that I know you are open to this PR, I will do that.

"""
This logging middleware will log all requests as they go through.
They are, by default, sent to a logger named ``'wsgi'`` at the
INFO level.
If ``setup_console_handler`` is true, then messages for the named
logger will be sent to the console.
"""

format = (
"%(REMOTE_ADDR)s - %(REMOTE_USER)s [%(time)s] "
'"%(REQUEST_METHOD)s %(REQUEST_URI)s %(HTTP_VERSION)s" '
'%(status)s %(bytes)s "%(HTTP_REFERER)s" "%(HTTP_USER_AGENT)s"'
)

def __init__(
self,
application,
logger=None,
format=None,
logging_level=logging.INFO,
logger_name="wsgi",
setup_console_handler=True,
set_logger_level=logging.DEBUG,
):
if format is not None:
self.format = format
self.application = application
self.logging_level = logging_level
self.logger_name = logger_name
if logger is None:
self.logger = logging.getLogger(self.logger_name)
if setup_console_handler:
console = logging.StreamHandler()
console.setLevel(logging.DEBUG)
# We need to control the exact format:
console.setFormatter(logging.Formatter("%(message)s"))
self.logger.addHandler(console)
self.logger.propagate = False
if set_logger_level is not None:
self.logger.setLevel(set_logger_level)
else:
self.logger = logger

def __call__(self, environ, start_response):
start = time.localtime()
req_uri = urllib.parse.quote(
environ.get("SCRIPT_NAME", "") + environ.get("PATH_INFO", "")
)
if environ.get("QUERY_STRING"):
req_uri += "?" + environ["QUERY_STRING"]
method = environ["REQUEST_METHOD"]

def replacement_start_response(status, headers, exc_info=None):
# @@: Ideally we would count the bytes going by if no
# content-length header was provided; but that does add
# some overhead, so at least for now we'll be lazy.
bytes = None
for name, value in headers:
if name.lower() == "content-length":
bytes = value
self.write_log(environ, method, req_uri, start, status, bytes)
return start_response(status, headers)

return self.application(environ, replacement_start_response)

def write_log(self, environ, method, req_uri, start, status, bytes):
if bytes is None:
bytes = "-"
if time.daylight:
offset = time.altzone / 60 / 60 * -100
else:
offset = time.timezone / 60 / 60 * -100
if offset >= 0:
offset = "+%0.4d" % (offset)
elif offset < 0:
offset = "%0.4d" % (offset)
remote_addr = "-"
if environ.get("HTTP_X_FORWARDED_FOR"):
remote_addr = environ["HTTP_X_FORWARDED_FOR"]
elif environ.get("REMOTE_ADDR"):
remote_addr = environ["REMOTE_ADDR"]
d = {
"REMOTE_ADDR": remote_addr,
"REMOTE_USER": environ.get("REMOTE_USER") or "-",
"REQUEST_METHOD": method,
"REQUEST_URI": req_uri,
"HTTP_VERSION": environ.get("SERVER_PROTOCOL"),
"time": time.strftime("%d/%b/%Y:%H:%M:%S ", start) + offset,
"status": status.split(None, 1)[0],
"bytes": bytes,
"HTTP_REFERER": environ.get("HTTP_REFERER", "-"),
"HTTP_USER_AGENT": environ.get("HTTP_USER_AGENT", "-"),
}
message = self.format % d
self.logger.log(self.logging_level, message)


def make_filter(
app,
global_conf,
logger_name="wsgi",
format=None,
logging_level=logging.INFO,
setup_console_handler=True,
set_logger_level=logging.DEBUG,
):
from paste.util.converters import asbool

if isinstance(logging_level, (bytes, str)):
logging_level = logging._levelNames[logging_level]
if isinstance(set_logger_level, (bytes, str)):
set_logger_level = logging._levelNames[set_logger_level]
return TransLogger(
app,
format=format or None,
logging_level=logging_level,
logger_name=logger_name,
setup_console_handler=asbool(setup_console_handler),
set_logger_level=set_logger_level,
)


make_filter.__doc__ = TransLogger.__doc__
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ dependencies = [
"Flask-Limiter>=2.9.0",
"Flask-SQLAlchemy",
"marshmallow>=3.13.0",
"waitress",
"webargs",
"SQLAlchemy",
"pdf2image",
Expand Down