Re: [PATCH v3 2/2] Introduce conf/config.dev for development
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!
[PATCH v3 2/2] Introduce conf/config.dev for development
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 ..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
Re: [PATCH v2 2/2] Introduce conf/config.dev for development
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.
Re: [PATCH v2 1/5] refactor code to comply with flake8 and isort
Nit: We usually capitalize the first word in commit messages. On Mon, 01 Jun 2020 at 18:35:25, Filipe Laíns wrote: > Signed-off-by: Filipe Laíns > --- > aurweb/git/auth.py | 3 +- > aurweb/git/serve.py | 14 +- > aurweb/git/update.py| 6 +- > aurweb/initdb.py| 7 +- > aurweb/l10n.py | 2 +- > aurweb/schema.py| 4 +- > aurweb/scripts/aurblup.py | 3 +- > aurweb/scripts/rendercomment.py | 6 +- > migrations/env.py | 9 +- > schema/gendummydata.py | 354 > setup.py| 3 +- > 11 files changed, 210 insertions(+), 201 deletions(-) Merged into pu, thanks!
Re: [PATCH v2 2/2] Introduce conf/config.dev for development
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!
Re: Tech stack for Python aurweb
Thank you for your comments Ricardo and Baptiste. On Tue, 02 Jun 2020 at 09:07:21, Baptiste Jonglez wrote: > I generally don't like the kind of comments that go "it would be nicer if > you do X and Y" from people that won't actually participate, but I still > feel it's relevant: One of the reasons for starting this thread was to give non-core developers the opportunity to step in and make alternative proposals or raise concerns, so no need to worry about that. > On 24-05-20, Ricardo Band wrote: > > I'm a python programmer for a while now and I really like the small > > size and simplicity of Flask. But let me give you one advice here. > > If you're dealing with a more complex application Flask tends to become > > more complex and very hard to manage. All of a sudden you integrate > > about 10 Flask addons. Some of them are not well maintained. Some > > droppen support for others etc. > > You have to manage all those dependencies. > > > > Personally I prefer Django in more complex projects as it comes with > > everything included. You rarely have to add something to your > > dependencies. > > I completely agree with this. > > Flask is fantastic for simple projects. But once you start adding users, > authentication, an admin panel, command-line scripts, and so on, it > quickly becomes a mess. Django is much more structured, which is a big > advantage for complex projects. One thing I should have clarified much more in my first email in this thread is that this is part of a dual stack solution, with most of the code still being written in PHP and only specific pages being handled by the new framework. Everything we do (including database access, user and session management, ...) must be fully compatible with our PHP implementation, hence we would almost certainly not be able to use any of the more sophisticated features that these more comprehensive frameworks provide. > Regarding databases and migrations, Django is much nicer to work with than > SQLAlchemy / Flask-SQLAlchemy. For instance, with Flask-SQLAlchemy I've > had serious consistency issues between PostgreSQL and Sqlite, something > that I had never seen with Django. Same here, everything needs to be compatible with our existing PHP code base; and we're already using SQLAlchemy for migrations there (even though this would be not too hard to change at this point if there is a strong reason for doing so). > Also, the Flask ecosystem is not exactly bustling with activity. On the > positive side, this is because things are remarkably stable and working as > intended. On the negative side, it means some amount of bit-rot and > unmaintained projects, although it's clearly not to the point of complete > disrepair. As you may have noticed from some of the patches submitted to the ML, we decided to use FastAPI over Flask. Best regards, Lukas
Re: Tech stack for Python aurweb
Hi, While I don't plan to work on the new AUR, I have experience with both Flask and Django. I generally don't like the kind of comments that go "it would be nicer if you do X and Y" from people that won't actually participate, but I still feel it's relevant: On 24-05-20, Ricardo Band wrote: > I'm a python programmer for a while now and I really like the small > size and simplicity of Flask. But let me give you one advice here. > If you're dealing with a more complex application Flask tends to become > more complex and very hard to manage. All of a sudden you integrate > about 10 Flask addons. Some of them are not well maintained. Some > droppen support for others etc. > You have to manage all those dependencies. > > Personally I prefer Django in more complex projects as it comes with > everything included. You rarely have to add something to your > dependencies. I completely agree with this. Flask is fantastic for simple projects. But once you start adding users, authentication, an admin panel, command-line scripts, and so on, it quickly becomes a mess. Django is much more structured, which is a big advantage for complex projects. Regarding databases and migrations, Django is much nicer to work with than SQLAlchemy / Flask-SQLAlchemy. For instance, with Flask-SQLAlchemy I've had serious consistency issues between PostgreSQL and Sqlite, something that I had never seen with Django. Also, the Flask ecosystem is not exactly bustling with activity. On the positive side, this is because things are remarkably stable and working as intended. On the negative side, it means some amount of bit-rot and unmaintained projects, although it's clearly not to the point of complete disrepair. > My personal rule of thumb is to use Flask for everything simple. When > users are involved it immediatly becomes complex and I switch to > Django. Very nice rule of thumb indeed! > So for example a small API without user database is good job for flask. > A blog like app where I have to deal with users, registrations etc is a > good job for Django. With all that being said, if the people that will be developping the new AUR don't know anything about Django, it would mean investing time to learn Django. Baptiste signature.asc Description: PGP signature
Re: [PATCH v2 2/2] Introduce conf/config.dev for development
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 ..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!
