[PATCH] aurweb.spawn: Integrate FastAPI and nginx
aurweb.spawn used to launch only PHP’s built-in server. Now it spawns a dummy FastAPI application too. Since both stacks spawn their own HTTP server, aurweb.spawn also spawns nginx as a reverse proxy to mount them under the same base URL, defined by aur_location in the configuration. --- TESTING | 3 +- aurweb/asgi.py | 8 +++++ aurweb/spawn.py | 80 +++++++++++++++++++++++++++++++++++++------- conf/config.defaults | 7 ++++ 4 files changed, 85 insertions(+), 13 deletions(-) create mode 100644 aurweb/asgi.py diff --git a/TESTING b/TESTING index a5e08cb8..31e3bcbd 100644 --- a/TESTING +++ b/TESTING @@ -12,7 +12,8 @@ INSTALL. 2) Install the necessary packages: # pacman -S --needed php php-sqlite sqlite words fortune-mod \ - python python-sqlalchemy python-alembic + python python-sqlalchemy python-alembic \ + python-fastapi uvicorn nginx Ensure to enable the pdo_sqlite extension in php.ini. diff --git a/aurweb/asgi.py b/aurweb/asgi.py new file mode 100644 index 00000000..8d3deedc --- /dev/null +++ b/aurweb/asgi.py @@ -0,0 +1,8 @@ +from fastapi import FastAPI + +app = FastAPI() + + +@app.get("/sso/") +async def hello(): + return {"message": "Hello from FastAPI!"} diff --git a/aurweb/spawn.py b/aurweb/spawn.py index 5fa646b5..df6a2ed5 100644 --- a/aurweb/spawn.py +++ b/aurweb/spawn.py @@ -10,8 +10,10 @@ configuration anyway. import atexit import argparse +import os import subprocess import sys +import tempfile import time import urllib @@ -20,6 +22,7 @@ import aurweb.schema children = [] +temporary_dir = None verbosity = 0 @@ -35,10 +38,42 @@ class ProcessExceptions(Exception): super().__init__("\n- ".join(messages)) +def generate_nginx_config(): + """ + Generate an nginx configuration based on aurweb's configuration. + The file is generated under `temporary_dir`. + Returns the path to the created configuration file. + """ + aur_location = aurweb.config.get("options", "aur_location") + aur_location_parts = urllib.parse.urlsplit(aur_location) + config_path = os.path.join(temporary_dir, "nginx.conf") + config = open(config_path, "w") + # We double nginx's braces because they conflict with Python's f-strings. + config.write(f""" + events {{}} + daemon off; + error_log /dev/stderr info; + pid {os.path.join(temporary_dir, "nginx.pid")}; + http {{ + access_log /dev/stdout; + server {{ + listen {aur_location_parts.netloc}; + location / {{ + proxy_pass http://{aurweb.config.get("php", "bind_address")}; + }} + location /sso {{ + proxy_pass http://{aurweb.config.get("fastapi", "bind_address")}; + }} + }} + }} + """) + return config_path + + def spawn_child(args): """Open a subprocess and add it to the global state.""" if verbosity >= 1: - print(f"Spawning {args}", file=sys.stderr) + print(f":: Spawning {args}", file=sys.stderr) children.append(subprocess.Popen(args)) @@ -52,10 +87,29 @@ def start(): if children: return atexit.register(stop) - aur_location = aurweb.config.get("options", "aur_location") - aur_location_parts = urllib.parse.urlsplit(aur_location) - htmldir = aurweb.config.get("options", "htmldir") - spawn_child(["php", "-S", aur_location_parts.netloc, "-t", htmldir]) + + # Friendly message + ruler = "-" * os.get_terminal_size().columns + print(ruler) + print("Spawing PHP and FastAPI, then nginx as a reverse proxy.") + print("Check out " + aurweb.config.get("options", "aur_location")) + print("Hit ^C to terminate everything.") + print(ruler) + + # PHP + php_address = aurweb.config.get("php", "bind_address") + htmldir = aurweb.config.get("php", "htmldir") + spawn_child(["php", "-S", php_address, "-t", htmldir]) + + # FastAPI + host, port = aurweb.config.get("fastapi", "bind_address").rsplit(":", 1) + spawn_child(["python", "-m", "uvicorn", + "--host", host, + "--port", port, + "aurweb.asgi:app"]) + + # nginx + spawn_child(["nginx", "-p", temporary_dir, "-c", generate_nginx_config()]) def stop(): @@ -73,7 +127,7 @@ def stop(): try: p.terminate() if verbosity >= 1: - print(f"Sent SIGTERM to {p.args}", file=sys.stderr) + print(f":: Sent SIGTERM to {p.args}", file=sys.stderr) except Exception as e: exceptions.append(e) for p in children: @@ -99,9 +153,11 @@ if __name__ == '__main__': help='increase verbosity') args = parser.parse_args() verbosity = args.verbose - start() - try: - while True: - time.sleep(60) - except KeyboardInterrupt: - stop() + with tempfile.TemporaryDirectory(prefix="aurweb-") as tmpdirname: + temporary_dir = tmpdirname + start() + try: + while True: + time.sleep(60) + except KeyboardInterrupt: + stop() diff --git a/conf/config.defaults b/conf/config.defaults index 86fe765c..ed495168 100644 --- a/conf/config.defaults +++ b/conf/config.defaults @@ -41,9 +41,16 @@ cache = none cache_pkginfo_ttl = 86400 memcache_servers = 127.0.0.1:11211 +[php] +; Address PHP should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8081 ; Directory containing aurweb's PHP code, required by aurweb.spawn. ;htmldir = /path/to/web/html +[fastapi] +; Address uvicorn should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8082 + [ratelimit] request_limit = 4000 window_length = 86400 -- 2.26.2
On Sat, 2020-05-30 at 19:41 +0200, Frédéric Mangano-Tarumi wrote:
aurweb.spawn used to launch only PHP’s built-in server. Now it spawns a dummy FastAPI application too. Since both stacks spawn their own HTTP server, aurweb.spawn also spawns nginx as a reverse proxy to mount them under the same base URL, defined by aur_location in the configuration. --- TESTING | 3 +- aurweb/asgi.py | 8 +++++ aurweb/spawn.py | 80 +++++++++++++++++++++++++++++++++++++------- conf/config.defaults | 7 ++++ 4 files changed, 85 insertions(+), 13 deletions(-) create mode 100644 aurweb/asgi.py
diff --git a/TESTING b/TESTING index a5e08cb8..31e3bcbd 100644 --- a/TESTING +++ b/TESTING @@ -12,7 +12,8 @@ INSTALL. 2) Install the necessary packages:
# pacman -S --needed php php-sqlite sqlite words fortune-mod \ - python python-sqlalchemy python-alembic + python python-sqlalchemy python-alembic \ + python-fastapi uvicorn nginx
Add this to .gitlab-ci.yml too.
Ensure to enable the pdo_sqlite extension in php.ini.
diff --git a/aurweb/asgi.py b/aurweb/asgi.py new file mode 100644 index 00000000..8d3deedc --- /dev/null +++ b/aurweb/asgi.py @@ -0,0 +1,8 @@ +from fastapi import FastAPI + +app = FastAPI() + + +@app.get("/sso/") +async def hello(): + return {"message": "Hello from FastAPI!"} diff --git a/aurweb/spawn.py b/aurweb/spawn.py index 5fa646b5..df6a2ed5 100644 --- a/aurweb/spawn.py +++ b/aurweb/spawn.py @@ -10,8 +10,10 @@ configuration anyway.
import atexit import argparse +import os import subprocess import sys +import tempfile import time import urllib
@@ -20,6 +22,7 @@ import aurweb.schema
children = [] +temporary_dir = None verbosity = 0
@@ -35,10 +38,42 @@ class ProcessExceptions(Exception): super().__init__("\n- ".join(messages))
+def generate_nginx_config(): + """ + Generate an nginx configuration based on aurweb's configuration. + The file is generated under `temporary_dir`. + Returns the path to the created configuration file. + """ + aur_location = aurweb.config.get("options", "aur_location") + aur_location_parts = urllib.parse.urlsplit(aur_location) + config_path = os.path.join(temporary_dir, "nginx.conf") + config = open(config_path, "w") + # We double nginx's braces because they conflict with Python's f-strings. + config.write(f""" + events {{}} + daemon off; + error_log /dev/stderr info; + pid {os.path.join(temporary_dir, "nginx.pid")}; + http {{ + access_log /dev/stdout; + server {{ + listen {aur_location_parts.netloc}; + location / {{ + proxy_pass http://{aurweb.config.get("php", "bind_address")}; + }} + location /sso {{ + proxy_pass http://{aurweb.config.get("fastapi", "bind_address")}; + }} + }} + }} + """) + return config_path + + def spawn_child(args): """Open a subprocess and add it to the global state.""" if verbosity >= 1: - print(f"Spawning {args}", file=sys.stderr) + print(f":: Spawning {args}", file=sys.stderr) children.append(subprocess.Popen(args))
@@ -52,10 +87,29 @@ def start(): if children: return atexit.register(stop) - aur_location = aurweb.config.get("options", "aur_location") - aur_location_parts = urllib.parse.urlsplit(aur_location) - htmldir = aurweb.config.get("options", "htmldir") - spawn_child(["php", "-S", aur_location_parts.netloc, "-t", htmldir]) + + # Friendly message + ruler = "-" * os.get_terminal_size().columns + print(ruler) + print("Spawing PHP and FastAPI, then nginx as a reverse proxy.") + print("Check out " + aurweb.config.get("options", "aur_location")) + print("Hit ^C to terminate everything.") + print(ruler)
I would have a single print statement and replace ^C with CTRL+C. But this is just a nitpick.
+ + # PHP + php_address = aurweb.config.get("php", "bind_address") + htmldir = aurweb.config.get("php", "htmldir") + spawn_child(["php", "-S", php_address, "-t", htmldir]) + + # FastAPI + host, port = aurweb.config.get("fastapi", "bind_address").rsplit(":", 1) + spawn_child(["python", "-m", "uvicorn", + "--host", host, + "--port", port, + "aurweb.asgi:app"]) + + # nginx + spawn_child(["nginx", "-p", temporary_dir, "-c", generate_nginx_config()])
def stop(): @@ -73,7 +127,7 @@ def stop(): try: p.terminate() if verbosity >= 1: - print(f"Sent SIGTERM to {p.args}", file=sys.stderr) + print(f":: Sent SIGTERM to {p.args}", file=sys.stderr) except Exception as e: exceptions.append(e) for p in children: @@ -99,9 +153,11 @@ if __name__ == '__main__': help='increase verbosity') args = parser.parse_args() verbosity = args.verbose - start() - try: - while True: - time.sleep(60) - except KeyboardInterrupt: - stop() + with tempfile.TemporaryDirectory(prefix="aurweb-") as tmpdirname: + temporary_dir = tmpdirname + start() + try: + while True: + time.sleep(60) + except KeyboardInterrupt: + stop() diff --git a/conf/config.defaults b/conf/config.defaults index 86fe765c..ed495168 100644 --- a/conf/config.defaults +++ b/conf/config.defaults @@ -41,9 +41,16 @@ cache = none cache_pkginfo_ttl = 86400 memcache_servers = 127.0.0.1:11211
+[php] +; Address PHP should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8081 ; Directory containing aurweb's PHP code, required by aurweb.spawn. ;htmldir = /path/to/web/html
+[fastapi] +; Address uvicorn should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8082 + [ratelimit] request_limit = 4000 window_length = 86400
Otherwise, LGTM. Filipe Laíns
Thanks a lot for the patch; two minor comments inline in addition to what Filipe mentioned previously (I am fine with ^C instead of CTRL+C by the way but don't care much either way). On Sat, 30 May 2020 at 13:41:07, Frédéric Mangano-Tarumi wrote:
aurweb.spawn used to launch only PHP\u2019s built-in server. Now it spawns a dummy FastAPI application too. Since both stacks spawn their own HTTP server, aurweb.spawn also spawns nginx as a reverse proxy to mount them under the same base URL, defined by aur_location in the configuration. --- TESTING | 3 +- aurweb/asgi.py | 8 +++++ aurweb/spawn.py | 80 +++++++++++++++++++++++++++++++++++++------- conf/config.defaults | 7 ++++ 4 files changed, 85 insertions(+), 13 deletions(-) create mode 100644 aurweb/asgi.py [...] diff --git a/aurweb/asgi.py b/aurweb/asgi.py new file mode 100644 index 00000000..8d3deedc --- /dev/null +++ b/aurweb/asgi.py @@ -0,0 +1,8 @@ +from fastapi import FastAPI + +app = FastAPI() + + +@app.get("/sso/") +async def hello(): + return {"message": "Hello from FastAPI!"}
I know that this is in preparation of the next step, SSO, but can we use a separate path here, such as /hello/ or /test/? That would cause less confusion to future readers of this patch (who may not have any context) and would allow us to decouple the actual SSO patch series from this patch more easily. For example, that series could start with a patch to remove this test, and all followup patches would not mention this test at all.
diff --git a/conf/config.defaults b/conf/config.defaults index 86fe765c..ed495168 100644 --- a/conf/config.defaults +++ b/conf/config.defaults @@ -41,9 +41,16 @@ cache = none cache_pkginfo_ttl = 86400 memcache_servers = 127.0.0.1:11211
+[php] +; Address PHP should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8081 ; Directory containing aurweb's PHP code, required by aurweb.spawn. ;htmldir = /path/to/web/html
+[fastapi] +; Address uvicorn should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8082 +
Even if the dev-only options are indicated by comments, I am not sure whether it's a good idea to mix production and development configuration like this. Can we put them in a separate file or at least a separate section without too much effort? Best regards, Lukas
Lukas Fleischer [2020-05-31 10:16:43 -0400]
diff --git a/conf/config.defaults b/conf/config.defaults index 86fe765c..ed495168 100644 --- a/conf/config.defaults +++ b/conf/config.defaults @@ -41,9 +41,16 @@ cache = none cache_pkginfo_ttl = 86400 memcache_servers = 127.0.0.1:11211
+[php] +; Address PHP should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8081 ; Directory containing aurweb's PHP code, required by aurweb.spawn. ;htmldir = /path/to/web/html
+[fastapi] +; Address uvicorn should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8082 +
Even if the dev-only options are indicated by comments, I am not sure whether it's a good idea to mix production and development configuration like this. Can we put them in a separate file or at least a separate section without too much effort?
aurweb.config is quite trivial so we could easily add mechanisms for handling environments or things like that. However, since these values are just ignored by production, I don’t think it’s a big deal. How about creating conf/config.dev with dev-specific options, and options most developers need to change (disable_http_login, etc.). In TESTING, we would change 3) to “Copy conf/config.dev to conf/config”. The downside is that when we add new required options to config.dev, developers need to update their conf/config manually. A more powerful option would be to have conf.d/00-defaults.ini, conf.d/60-dev.ini, and conf.d/90-user.ini. I think most developers are familiar with conf.d configurations already. Such a switch would break everyone’s setup though, including production, but the fix is easy. What do you think?
On Sun, 31 May 2020 at 17:20:45, Frédéric Mangano-Tarumi wrote:
Even if the dev-only options are indicated by comments, I am not sure whether it's a good idea to mix production and development configuration like this. Can we put them in a separate file or at least a separate section without too much effort?
aurweb.config is quite trivial so we could easily add mechanisms for handling environments or things like that. However, since these values are just ignored by production, I don\u2019t think it\u2019s a big deal.
How about creating conf/config.dev with dev-specific options, and options most developers need to change (disable_http_login, etc.). In TESTING, we would change 3) to \u201cCopy conf/config.dev to conf/config\u201d. The downside is that when we add new required options to config.dev, developers need to update their conf/config manually.
With the current design, you already need to copy the config file to /etc/aurweb/ on changes or use the AUR_CONFIG environment variable to point to an alternative path (TESTING uses the latter). So maybe create a development version of the config file as you suggested, add the dev-only options only there and update the TESTING instructions to use that file instead of the production config defaults?
aurweb.spawn used to launch only PHP’s built-in server. Now it spawns a dummy FastAPI application too. Since both stacks spawn their own HTTP server, aurweb.spawn also spawns nginx as a reverse proxy to mount them under the same base URL, defined by aur_location in the configuration. --- .gitlab-ci.yml | 2 +- TESTING | 3 +- aurweb/asgi.py | 8 +++++ aurweb/spawn.py | 80 +++++++++++++++++++++++++++++++++++++------- conf/config.defaults | 7 ++++ 5 files changed, 86 insertions(+), 14 deletions(-) create mode 100644 aurweb/asgi.py diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 74784fce..f6260ebb 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -11,7 +11,7 @@ before_script: base-devel git gpgme protobuf pyalpm python-mysql-connector python-pygit2 python-srcinfo python-bleach python-markdown python-sqlalchemy python-alembic python-pytest python-werkzeug - python-pytest-tap + python-pytest-tap python-fastapi uvicorn nginx test: script: diff --git a/TESTING b/TESTING index a5e08cb8..31e3bcbd 100644 --- a/TESTING +++ b/TESTING @@ -12,7 +12,8 @@ INSTALL. 2) Install the necessary packages: # pacman -S --needed php php-sqlite sqlite words fortune-mod \ - python python-sqlalchemy python-alembic + python python-sqlalchemy python-alembic \ + python-fastapi uvicorn nginx Ensure to enable the pdo_sqlite extension in php.ini. diff --git a/aurweb/asgi.py b/aurweb/asgi.py new file mode 100644 index 00000000..5f30471a --- /dev/null +++ b/aurweb/asgi.py @@ -0,0 +1,8 @@ +from fastapi import FastAPI + +app = FastAPI() + + +@app.get("/hello/") +async def hello(): + return {"message": "Hello from FastAPI!"} diff --git a/aurweb/spawn.py b/aurweb/spawn.py index 5fa646b5..0506afa4 100644 --- a/aurweb/spawn.py +++ b/aurweb/spawn.py @@ -10,8 +10,10 @@ configuration anyway. import atexit import argparse +import os import subprocess import sys +import tempfile import time import urllib @@ -20,6 +22,7 @@ import aurweb.schema children = [] +temporary_dir = None verbosity = 0 @@ -35,10 +38,42 @@ class ProcessExceptions(Exception): super().__init__("\n- ".join(messages)) +def generate_nginx_config(): + """ + Generate an nginx configuration based on aurweb's configuration. + The file is generated under `temporary_dir`. + Returns the path to the created configuration file. + """ + aur_location = aurweb.config.get("options", "aur_location") + aur_location_parts = urllib.parse.urlsplit(aur_location) + config_path = os.path.join(temporary_dir, "nginx.conf") + config = open(config_path, "w") + # We double nginx's braces because they conflict with Python's f-strings. + config.write(f""" + events {{}} + daemon off; + error_log /dev/stderr info; + pid {os.path.join(temporary_dir, "nginx.pid")}; + http {{ + access_log /dev/stdout; + server {{ + listen {aur_location_parts.netloc}; + location / {{ + proxy_pass http://{aurweb.config.get("php", "bind_address")}; + }} + location /hello {{ + proxy_pass http://{aurweb.config.get("fastapi", "bind_address")}; + }} + }} + }} + """) + return config_path + + def spawn_child(args): """Open a subprocess and add it to the global state.""" if verbosity >= 1: - print(f"Spawning {args}", file=sys.stderr) + print(f":: Spawning {args}", file=sys.stderr) children.append(subprocess.Popen(args)) @@ -52,10 +87,29 @@ def start(): if children: return atexit.register(stop) - aur_location = aurweb.config.get("options", "aur_location") - aur_location_parts = urllib.parse.urlsplit(aur_location) - htmldir = aurweb.config.get("options", "htmldir") - spawn_child(["php", "-S", aur_location_parts.netloc, "-t", htmldir]) + + print("{ruler}\n" + "Spawing PHP and FastAPI, then nginx as a reverse proxy.\n" + "Check out {aur_location}\n" + "Hit ^C to terminate everything.\n" + "{ruler}" + .format(ruler=("-" * os.get_terminal_size().columns), + aur_location=aurweb.config.get('options', 'aur_location'))) + + # PHP + php_address = aurweb.config.get("php", "bind_address") + htmldir = aurweb.config.get("php", "htmldir") + spawn_child(["php", "-S", php_address, "-t", htmldir]) + + # FastAPI + host, port = aurweb.config.get("fastapi", "bind_address").rsplit(":", 1) + spawn_child(["python", "-m", "uvicorn", + "--host", host, + "--port", port, + "aurweb.asgi:app"]) + + # nginx + spawn_child(["nginx", "-p", temporary_dir, "-c", generate_nginx_config()]) def stop(): @@ -73,7 +127,7 @@ def stop(): try: p.terminate() if verbosity >= 1: - print(f"Sent SIGTERM to {p.args}", file=sys.stderr) + print(f":: Sent SIGTERM to {p.args}", file=sys.stderr) except Exception as e: exceptions.append(e) for p in children: @@ -99,9 +153,11 @@ if __name__ == '__main__': help='increase verbosity') args = parser.parse_args() verbosity = args.verbose - start() - try: - while True: - time.sleep(60) - except KeyboardInterrupt: - stop() + with tempfile.TemporaryDirectory(prefix="aurweb-") as tmpdirname: + temporary_dir = tmpdirname + start() + try: + while True: + time.sleep(60) + except KeyboardInterrupt: + stop() diff --git a/conf/config.defaults b/conf/config.defaults index 86fe765c..ed495168 100644 --- a/conf/config.defaults +++ b/conf/config.defaults @@ -41,9 +41,16 @@ cache = none cache_pkginfo_ttl = 86400 memcache_servers = 127.0.0.1:11211 +[php] +; Address PHP should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8081 ; Directory containing aurweb's PHP code, required by aurweb.spawn. ;htmldir = /path/to/web/html +[fastapi] +; Address uvicorn should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8082 + [ratelimit] request_limit = 4000 window_length = 86400 -- 2.26.2
conf/config.dev’s purpose is to provide a lighter configuration template for developers, and split development-specific options off the default configuration file. --- TESTING | 8 +++----- conf/config.defaults | 10 ---------- conf/config.dev | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 15 deletions(-) create mode 100644 conf/config.dev diff --git a/TESTING b/TESTING index 31e3bcbd..f62baefa 100644 --- a/TESTING +++ b/TESTING @@ -17,12 +17,10 @@ INSTALL. Ensure to enable the pdo_sqlite extension in php.ini. -3) Copy conf/config.defaults to conf/config and adjust the configuration - Pay attention to disable_http_login, enable_maintenance, aur_location and - htmldir. +3) Copy conf/config.dev to conf/config and read its instructions. - Be sure to change backend to sqlite and name to the file location of your - created test database. + Note that when the upstream config.dev is updated, you should compare it to + your conf/config. 4) Prepare the testing database: diff --git a/conf/config.defaults b/conf/config.defaults index ed495168..447dacac 100644 --- a/conf/config.defaults +++ b/conf/config.defaults @@ -41,16 +41,6 @@ cache = none cache_pkginfo_ttl = 86400 memcache_servers = 127.0.0.1:11211 -[php] -; Address PHP should bind when spawned in development mode by aurweb.spawn. -bind_address = 127.0.0.1:8081 -; Directory containing aurweb's PHP code, required by aurweb.spawn. -;htmldir = /path/to/web/html - -[fastapi] -; Address uvicorn should bind when spawned in development mode by aurweb.spawn. -bind_address = 127.0.0.1:8082 - [ratelimit] request_limit = 4000 window_length = 86400 diff --git a/conf/config.dev b/conf/config.dev new file mode 100644 index 00000000..e9c2112e --- /dev/null +++ b/conf/config.dev @@ -0,0 +1,36 @@ +; Configuration file for aurweb development. +; +; Options are implicitly inherited from ${AUR_CONFIG}.defaults, which lists all +; available options for productions, and their default values. This current file +; overrides only options useful for development, and introduces +; development-specific options too. +; +; Paths need to be absolute, so please replace $AUR_ROOT with your project root. + +[database] + +backend = sqlite +name = $AUR_ROOT/aurweb.sqlite3 + +; Alternative MySQL configuration +;backend = mysql +;name = aurweb +;user = aur +;password = aur + +[options] +aur_location = http://127.0.0.1:8080 +disable_http_login = 0 +enable-maintenance = 0 + +[php] +; Address PHP should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8081 +; Directory containing aurweb's PHP code, required by aurweb.spawn. +htmldir = $AUR_ROOT/web/html + +[fastapi] +; Address uvicorn should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8082 + +; vim: ft=dosini -- 2.26.2
On Mon, 01 Jun 2020 at 12:50:23, Frédéric Mangano-Tarumi wrote:
conf/config.dev\u2019s purpose is to provide a lighter configuration template for developers, and split development-specific options off the default configuration file. --- TESTING | 8 +++----- conf/config.defaults | 10 ---------- conf/config.dev | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 15 deletions(-) create mode 100644 conf/config.dev
Great, thanks! A few remarks below.
diff --git a/conf/config.dev b/conf/config.dev new file mode 100644 index 00000000..e9c2112e --- /dev/null +++ b/conf/config.dev @@ -0,0 +1,36 @@ +; Configuration file for aurweb development. +; +; Options are implicitly inherited from ${AUR_CONFIG}.defaults, which lists all
Saying ${AUR_CONFIG}.defaults here is elegant, short, precise and clear to everybody who is familiar with the code base. However, given that those comments are mainly relevant to new contributors setting up a dev environment, I am not sure whether mentioning $AUR_CONFIG is a good idea here (people might not be aware of what exactly it is used for at this point). How about just saying config.defaults, given that the file is in the same directory? While being less precise, most people should still understand which file is referred to.
+; available options for productions, and their default values. This current file +; overrides only options useful for development, and introduces +; development-specific options too. +; +; Paths need to be absolute, so please replace $AUR_ROOT with your project root.
Even though you added a comment is here, I am not sure that using $AUR_ROOT below is a great idea since it is too easy to skip part of the comment and then think that $AUR_ROOT is an actual placeholder parsed by aurweb. I would either go back to putting a sensible default path everywhere or replicate this comment directly above all lines containing $AUR_ROOT.
+ +[database] +
Stray newline?
+backend = sqlite +name = $AUR_ROOT/aurweb.sqlite3 + +; Alternative MySQL configuration +;backend = mysql +;name = aurweb +;user = aur +;password = aur + +[options] +aur_location = http://127.0.0.1:8080 +disable_http_login = 0 +enable-maintenance = 0 + +[php] +; Address PHP should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8081 +; Directory containing aurweb's PHP code, required by aurweb.spawn. +htmldir = $AUR_ROOT/web/html + +[fastapi] +; Address uvicorn should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8082 + +; vim: ft=dosini
I think we usually don't add modelines or any other editor-specific configuration.
Lukas Fleischer [2020-06-01 21:05:57 -0400]
On Mon, 01 Jun 2020 at 12:50:23, Frédéric Mangano-Tarumi wrote:
diff --git a/conf/config.dev b/conf/config.dev new file mode 100644 index 00000000..e9c2112e --- /dev/null +++ b/conf/config.dev @@ -0,0 +1,36 @@ +; Configuration file for aurweb development. +; +; Options are implicitly inherited from ${AUR_CONFIG}.defaults, which lists all
Saying ${AUR_CONFIG}.defaults here is elegant, short, precise and clear to everybody who is familiar with the code base. However, given that those comments are mainly relevant to new contributors setting up a dev environment, I am not sure whether mentioning $AUR_CONFIG is a good idea here (people might not be aware of what exactly it is used for at this point). How about just saying config.defaults, given that the file is in the same directory? While being less precise, most people should still understand which file is referred to.
Developers need to set AUR_CONFIG to spawn aurweb. If we introduced it earlier like “Define the path to you configuration: export AUR_CONFIG="…"”, I think it should fit. As you implied though, using anything other than conf/config is madness. How about making aurweb.spawn set AUR_CONFIG to, by order of preference: 1. $AUR_CONFIG, if already defined, 2. $PWD/conf/config, if it exists, 3. $parent/conf/config where $parent is the nearest parent directory such that the file exists. aurweb.spawn is only meant for development, and it spawns test servers, so I think it makes sense to use a different default for AUR_CONFIG than production.
+; available options for productions, and their default values. This current file +; overrides only options useful for development, and introduces +; development-specific options too. +; +; Paths need to be absolute, so please replace $AUR_ROOT with your project root.
Even though you added a comment is here, I am not sure that using $AUR_ROOT below is a great idea since it is too easy to skip part of the comment and then think that $AUR_ROOT is an actual placeholder parsed by aurweb. I would either go back to putting a sensible default path everywhere or replicate this comment directly above all lines containing $AUR_ROOT.
I considered using relative paths, but because PHP changes the working directory and Python doesn’t, that can’t work. Nor can we have a sensible absolute path for everyone, since we cannot tell where the developer cloned the project. I can replace $AUR_ROOT by YOUR_AUR_ROOT, which is more explicit, and remove the comment. In TESTING, we can suggest using the following command: sed -e "s;YOUR_AUR_ROOT;$PWD;g" conf/config.dev > conf/config With more motivation, we could add support for environment variable interpolation in INI files. aurweb.config would know AUR_ROOT as a by-product of finding AUR_CONFIG, if we implement that.
+ +[database] +
Stray newline?
+backend = sqlite +name = $AUR_ROOT/aurweb.sqlite3 + +; Alternative MySQL configuration +;backend = mysql +;name = aurweb +;user = aur +;password = aur
I meant it to make the SQLite block stand out next to MySQL, but it does look special compared to other sections. In config.defaults, we use newlines only between sections, but that doesn’t leave much room for documentation. Interleaving comments with variables like I did below is okay only because the section is tiny. If the file grew bigger, I wish the norm was “block of documentation + set of correlated variable definitions + new line”. Sections would need a special comment to make them visible, like a ruler or a frame.
+[php] +; Address PHP should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8081 +; Directory containing aurweb's PHP code, required by aurweb.spawn. +htmldir = $AUR_ROOT/web/html
+; vim: ft=dosini
I think we usually don't add modelines or any other editor-specific configuration.
Ok!
On Tue, 02 Jun 2020 at 08:41:28, Frédéric Mangano-Tarumi wrote:
Saying ${AUR_CONFIG}.defaults here is elegant, short, precise and clear to everybody who is familiar with the code base. However, given that those comments are mainly relevant to new contributors setting up a dev environment, I am not sure whether mentioning $AUR_CONFIG is a good idea here (people might not be aware of what exactly it is used for at this point). How about just saying config.defaults, given that the file is in the same directory? While being less precise, most people should still understand which file is referred to.
Developers need to set AUR_CONFIG to spawn aurweb. If we introduced it earlier like \u201cDefine the path to you configuration: export AUR_CONFIG="\u2026"\u201d, I think it should fit.
That's better, even though I still don't believe it's more useful than just saying, config.defaults here. New users might set things up, then go back to the config the next day without knowing about $AUR_CONFIG anymore.
As you implied though, using anything other than conf/config is madness. How about making aurweb.spawn set AUR_CONFIG to, by order of preference:
1. $AUR_CONFIG, if already defined, 2. $PWD/conf/config, if it exists, 3. $parent/conf/config where $parent is the nearest parent directory such that the file exists.
aurweb.spawn is only meant for development, and it spawns test servers, so I think it makes sense to use a different default for AUR_CONFIG than production.
Sounds reasonable. I would prefer to infer the path from the location of the script itself (__file__) instead of (2) and (3) though.
I can replace $AUR_ROOT by YOUR_AUR_ROOT, which is more explicit, and remove the comment. In TESTING, we can suggest using the following command: sed -e "s;YOUR_AUR_ROOT;$PWD;g" conf/config.dev > conf/config
Sounds good.
With more motivation, we could add support for environment variable interpolation in INI\u202ffiles. aurweb.config would know AUR_ROOT as a by-product of finding AUR_CONFIG, if we implement that.
Yeah, probably not worthwhile for just this one use case but something we might want to look into in the future. I am not sure what "aurweb.config would know AUR_ROOT as a by-product of finding AUR_CONFIG" means exactly but note that AUR_CONFIG is usually not contained in the document root in production setups.
I meant it to make the SQLite block stand out next to MySQL, but it does look special compared to other sections.
In config.defaults, we use newlines only between sections, but that doesn\u2019t leave much room for documentation. Interleaving comments with variables like I did below is okay only because the section is tiny. If the file grew bigger, I wish the norm was \u201cblock of documentation + set of correlated variable definitions + new line\u201d. Sections would need a special comment to make them visible, like a ruler or a frame.
I would hope that each section would correspond to exactly one set of correlated variable definitions, so we would not need this. I am aware that that's currently not the case though.
I think we usually don't add modelines or any other editor-specific configuration.
Ok!
Thanks!
Lukas Fleischer [2020-06-02 17:40:23 -0400]
On Tue, 02 Jun 2020 at 08:41:28, Frédéric Mangano-Tarumi wrote:
Developers need to set AUR_CONFIG to spawn aurweb. If we introduced it earlier like \u201cDefine the path to you configuration: export AUR_CONFIG="\u2026"\u201d, I think it should fit.
That's better, even though I still don't believe it's more useful than just saying, config.defaults here. New users might set things up, then go back to the config the next day without knowing about $AUR_CONFIG anymore.
Saying config.defaults makes it look like it’s hard-coded, without further help. However, I agree with you it’s the simplest option. I’d add an extra “See aurweb/config.py for details.” if you don’t mind.
As you implied though, using anything other than conf/config is madness. How about making aurweb.spawn set AUR_CONFIG to, by order of preference:
1. $AUR_CONFIG, if already defined, 2. $PWD/conf/config, if it exists, 3. $parent/conf/config where $parent is the nearest parent directory such that the file exists.
Sounds reasonable. I would prefer to infer the path from the location of the script itself (__file__) instead of (2) and (3) though.
I’m not sure about __file__ because it gets complicated if the package is installed in a venv or something. We won’t implement that now anyway, so let’s talk about it another day. Same for AUR_ROOT.
In config.defaults, we use newlines only between sections, but that doesn\u2019t leave much room for documentation. Interleaving comments with variables like I did below is okay only because the section is tiny. If the file grew bigger, I wish the norm was \u201cblock of documentation + set of correlated variable definitions + new line\u201d. Sections would need a special comment to make them visible, like a ruler or a frame.
I would hope that each section would correspond to exactly one set of correlated variable definitions, so we would not need this. I am aware that that's currently not the case though.
Actually I meant a stronger correlation, like variables that share the same documentation because that would make no sense to document them individually. See /etc/php/php.ini for the kind of layout I had in mind. There’s an empty line between every variable, and the file would be unreadable without it. I’ll apply that style but feel free to drop the empty lines from my patch without notice if you don’t like them.
conf/config.dev’s purpose is to provide a lighter configuration template for developers, and split development-specific options off the default configuration file. --- TESTING | 11 ++++++----- conf/config.defaults | 10 ---------- conf/config.dev | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 conf/config.dev diff --git a/TESTING b/TESTING index 31e3bcbd..7261df92 100644 --- a/TESTING +++ b/TESTING @@ -17,12 +17,13 @@ INSTALL. Ensure to enable the pdo_sqlite extension in php.ini. -3) Copy conf/config.defaults to conf/config and adjust the configuration - Pay attention to disable_http_login, enable_maintenance, aur_location and - htmldir. +3) Copy conf/config.dev to conf/config and replace YOUR_AUR_ROOT by the absolute + path to the root of your aurweb clone. sed can do both tasks for you: - Be sure to change backend to sqlite and name to the file location of your - created test database. + $ sed -e "s;YOUR_AUR_ROOT;$PWD;g" conf/config.dev > conf/config + + Note that when the upstream config.dev is updated, you should compare it to + your conf/config, or regenerate your configuration with the command above. 4) Prepare the testing database: diff --git a/conf/config.defaults b/conf/config.defaults index ed495168..447dacac 100644 --- a/conf/config.defaults +++ b/conf/config.defaults @@ -41,16 +41,6 @@ cache = none cache_pkginfo_ttl = 86400 memcache_servers = 127.0.0.1:11211 -[php] -; Address PHP should bind when spawned in development mode by aurweb.spawn. -bind_address = 127.0.0.1:8081 -; Directory containing aurweb's PHP code, required by aurweb.spawn. -;htmldir = /path/to/web/html - -[fastapi] -; Address uvicorn should bind when spawned in development mode by aurweb.spawn. -bind_address = 127.0.0.1:8082 - [ratelimit] request_limit = 4000 window_length = 86400 diff --git a/conf/config.dev b/conf/config.dev new file mode 100644 index 00000000..d752f61f --- /dev/null +++ b/conf/config.dev @@ -0,0 +1,32 @@ +; Configuration file for aurweb development. +; +; Options are implicitly inherited from conf/config.defaults, which lists all +; available options for productions, and their default values. This current file +; overrides only options useful for development, and introduces +; development-specific options too. + +[database] +backend = sqlite +name = YOUR_AUR_ROOT/aurweb.sqlite3 + +; Alternative MySQL configuration +;backend = mysql +;name = aurweb +;user = aur +;password = aur + +[options] +aur_location = http://127.0.0.1:8080 +disable_http_login = 0 +enable-maintenance = 0 + +[php] +; Address PHP should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8081 + +; Directory containing aurweb's PHP code, required by aurweb.spawn. +htmldir = YOUR_AUR_ROOT/web/html + +[fastapi] +; Address uvicorn should bind when spawned in development mode by aurweb.spawn. +bind_address = 127.0.0.1:8082 -- 2.26.2
On Tue, 02 Jun 2020 at 20:04:02, Frédéric Mangano-Tarumi wrote:
conf/config.dev\u2019s purpose is to provide a lighter configuration template for developers, and split development-specific options off the default configuration file. --- TESTING | 11 ++++++----- conf/config.defaults | 10 ---------- conf/config.dev | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 15 deletions(-) create mode 100644 conf/config.dev
Looks good to me. Merged (together with patch 1/2), thanks!
participants (3)
-
Filipe Laíns
-
Frédéric Mangano-Tarumi
-
Lukas Fleischer