[PATCH 4 of 4] setup: add gearbox command to setup front-end

Tue Sep 18 19:47:24 UTC 2018

Hi Mads,
El dom., 16 sept. 2018 a las 22:51, Mads Kiilerich
(<mads at kiilerich.com>) escribió:
>
> [It seems like I failed to send this some days ago ... here it comes ...]
>
> On 09/11/2018 09:34 PM, Thomas De Schampheleire wrote:
> >>> The approach implemented by this commit is to add a custom gearbox command
> >>> 'setup-frontend' to run the required commands. This single user-facing
> >>> command can internally run various steps as needed. The only current
> >>> requirement is the presence of npm and an internet connection.
> >> What is this command really about, and what is the scope of it? I guess
> >> it essentially is something that prepare all the static files that can
> >> be served directly by the web server ... and perhaps even be served by a
> >> front-end server or put on a CDN?
> > If I take a step back, actually I would like the installation/setup of Kallithea
> > to be as easy as possible for the user. As such, the amount of commands should
> > be kept reasonable, and if it is not necessary to expose certain details to the
> > user (for example because there is no element of choice for it) then it should
> > rather be kept internal and run automatically in some way.
> >
> > Looking at the installation instructions from:
> > https://kallithea.readthedocs.io/en/default/installation.html
> > combined with required steps from:
> > https://kallithea.readthedocs.io/en/default/setup.html#setup
> > we get:
> >
> > 1.  hg clone https://kallithea-scm.org/repos/kallithea
> > 2.  cd kallithea
> > 3.  virtualenv ../kallithea-venv
> > 4.  . ../kallithea-venv/bin/activate
> > 5.  pip install --upgrade pip setuptools
> > 6.  pip install --upgrade -e .
> > 7.  python2 setup.py compile_catalog   # for translation of the UI
> > 8.  gearbox make-config my.ini
> > 9.  gearbox setup-db -c my.ini
> > 10. npm install
> > 11. npm run less
> >
> > Steps 1-5 cannot really be changed (also because virtualenv is not a
> > requirement).
> > Step 6 may be fine to expose to the user.
> > Step 7 is definitely something internal IMHO.
> > Step 8-9 need to be exposed because users may want to create different configs,
> > skip setting up the db if they already have one, etc.
> > Step 10-11 are more something internal.
> >
> > So, ideally I would like one command to group steps 7, 10 and 11 and possible
> > future steps needed to set up Kallithea.
>
> Nice perspective to look at it.
>
> Also, we must be careful to not have too much "magic" where users in
> actual use will end up with commands doing things they don't want.
>
> I think other import "dimensions" are:
> * should it only be run initially (totally the case for setup-db,
> usually the case for virtualenv and make-config (even though there can
> be exceptions when making big upgrades))
> * should it be run when upgrading (new application code,
> mandatory/optional installation/upgrade of pip/npm dependencies,
> front-end, database migration)
> * should it be run when setting up development environment (as initial
> setup, but also dev_requirements.txt, possibly localization files when
> installing from source)
> * should it be run frequently during development (mainly rebuilding
> front-end (as --reload and automatic template compilation fully take
> care of the rest), possibly debug mode for logging and (non)minification
> of front-end)
>
> (Some of these dimensions could perhaps also be exposed in the
> documentation to make it simpler and easier to understand.)
>
> So I agree that for normal users, the 2 npm steps (and pygmentize) (and
> localization if running from source?) could be bundled in one command.
> The "development" use case can perhaps have extra steps or special
> command options to use.
>
> > I'm not experienced in deployment scenarios where static files are put in other
> > places, so can't really comment on that.
> >
> >> Instead of augmenting the source directory, it should perhaps fully
> >> generate a target directory from scratch, populate it with npm output
> >> and copying additional files from the source tree. Neither Kallithea
> >> source directory nor npm installed stuff should be directly exposed by
> >> the web server.
> >>
> >> It should probably be possible to give the directory to the gearbox
> >> command (and/or configure it in the app config) so it doesn't have to
> >> clobber the site-packages directory but can be installed in a different
> >> place with different permissions and security posture.
> > Which gearbox command are you referring to here? 'serve' or 'setup-frontend'
> > ?
>
> setup-frontend. But I realize that in an installation where these
> generated files live outside the python location, then that directory
> should be specified in the .ini anyway ...
>
> > To explore this idea further (and to help my understanding), let's consider the
> > different ways to install and serve Kallithea. Please correct me where I'm
> > wrong.
> >
> > 1. Kallithea installed from source repo
> > Here the application is served directly from the repository,
> > the python dependencies are installed elsewhere (either in a virtualenv or from
> > a location in e.g. /usr).
> > the css and js of Kallithea itself is served from kallithea/public/ in the repo,
> > the css from npm dependencies are all accumulated in the big single css file
> > (via npm run less),
> > the js from npm dependencies are currently not used (e.g. bootstrap.js is still
> > coming from the repo itself).
> >
> > 2. Kallithea installed via pip
> > Here the application files are in the virtualenv or in /usr,
> > the css and js of Kallithea itself is served from kallithea/public/ from that
> > location in site-packages,
> > the css from npm dependencies are all accumulated in the big single css file
> > (via npm run less),
> > the js from npm dependencies are currently not used (e.g. bootstrap.js is still
> > coming from the repo itself).
> >
> > Are there other ways?
> > Whether you serve via gearbox, apache, uwsgi, etc. should not matter here,
> > right?
> >
> > So what you want to achieve with this suggestion, is to move the
> > kallithea/public directory to another place, chosen by the user (perhaps by
> > default inside the same directory where their ini file will be and the data
> > directory is?).
> > And in case any other than css/less files from npm are used, make sure they are
> > copied to that public directory too?
>
> Yes, pretty much.
>
> Essentially the observation that the "run from source" setup is the
> special case. In general, it might be problematic to assume that we can
> write to the "python location" when from a front-end step that is run
> after the "pip" (or rpm/deb/apt/...) step.
>
> Also, I think we should clarify whether kallithea/public actually is to
> be served publicly, or if it is source for generating stuff that should
> be served publicly. As we have to realize that it isn't feasible to ship
> the full "compiled" front-end code, I think we have to mainly consider
> it a "front-end source" directory (and perhaps rename it).
>
> The content of the static public directory will all be "generated",
> either by plan copy from the "front-end source" or generated with npm
> commands or pygmentize, etc.
>
> For now, we might want to keep or support the current "augment
> kallithea/public" approach. But it can easily be more confusing and
> cause extra work. We will have to copy around files around from npm
> packages anyway.
>
> >> Is "setup front-end" descriptive for what the command does? Is it really
> >> a "setup step"? Or is the role more similar to "make-config", and could
> >> be named "make-frontend"?
> > Current 'make' and 'setup' commands known to gearbox are:
> >
> >    make-config    Kallithea: Create a new config file
> >    make-index     Kallithea: Create or update full text search index
> >    make-rcext     Kallithea: Write template file for extending Kallithea in Python
> >    makepackage    Creates a basic python package
> >    setup-app      Setup an application, given a config file
> >    setup-db       Kallithea: Configure the database specified in the .ini file
> >
> > I consider 'setup' something more generic than 'make' in the sense that the word
> > 'setup' encompasses any step needed to get something ready, while 'make'
> > refers only to generation of something new.
> >
> > In particular given the comments earlier in this mail about also grouping other
> > stuff like compile_catalog, I would argue that 'setup' is better suited than
> > 'make', although the suffix 'front-end' becomes too restrictive.
> >
> > In fact, in the list is 'setup-app' which sounds suitable to me and as far as I
> > understand is not actually implemented yet. There is a default implementation in
> > gearbox itself but can probably be overwritten.
>
> `setup-app` is a default paster command that can't be hidden. I think it
> is unclear what it actually means to setup the app. Is it installing
> dependencies? Generating a config file? Creating the database or
> upgrading it, possibly losing data? Thus we use an explicit `setup-db`
> command. But that could all be subject to change. Possibly by having one
> top level setup-app command with different options / sub commands. (That
> also asks the question of what the actual value of using gearbox for our
> use case is.)

It is indeed clear what the intended scope of 'setup-app' in
paster/gearbox is, and overriding it may thus yield surprises to
users. Using another name may be better.

Regarding the value of gearbox for these commands: I indeed considered the same.
There exist other libraries that may be better suited for this type of
command with suboptions.
I haven't used it, but read positive feedback of Click:
http://click.pocoo.org/5/
You basically just write methods corresponding to the CLI commands you
need, and annotate them to expose them to users, possible with
options.

>
> >> I think there are valid use cases for separating the on-line "download
> >> and install npm stuff" part from an off-line "build from source and
> >> downloads" parts. Perhaps give the gearbox command options for just
> >> running one of these parts? I guess the on-line part is setup-ish, while
> >> the off-line build part is more make-ish ...
> > Yes, the download part may need more user/environment specific config like a
> > proxy or other settings for npm.
>
> For "serious" use, there might not even be internet access when
> installing. Things must be downloaded elsewhere first, then reviewed and
> copied over. Not a main use case, but if we have an answer to that use
> case (possibly just an idea of what amount of manual hacking it would
> require), then we have a good understanding of the problem and will
> cover the essentials.
>
> >>> For now, this will just create/update style.css ... but currently probably
> >>> without any actual changes.
> >> How will this gearbox approach work with other use cases as outlined on
> >> https://kallithea-scm.org/repos/kallithea/pull-request/137/_/remaining_bootstrap_related_stuff_v3#C--b9cfc7f2cdf7
> >> : Generated files (pygmentize) and automatic rebuild (kind of as server
> >> --reload) and minification and non-minified development mode (if
> >> relevant?). And JavaScript handling? Can we have a PoC that show how
> >> that could work?
> > pygmentize would be just an additional command to run during the 'setup-app' or
> > whatever we call it, right?
>
> Yes. It should ideally be run after upgrading the python package, or if
> the output for other reasons is needed.
>
> > For automatic rebuild: not sure. I had given in a comment on that PR a PoC to
> > watch .less and .css files from '--reload' but as Dominik pointed out it does
> > not cover the actual rebuild. So with this mechanism we could let the serve
> > reload when the css is regenerated (which is not actually needed), but the
> > generation itself would need to be triggered manually after the less is changed.
> > Alternatively we use something like nodemon to watch the less files and
> > generated the css, as originally proposed by Dominik.
>
> The '--reload' functionality also use a file watcher. We can do
> something similar in our "front-end build command".
>
> > For minification/no minification: I'm still not convinced that we need a choice
> > here. I'd let the command generate both minified and unminified, and let the
> > application use the minified one. Dominik said there may be problems when
> > debugging, but I'd argue that this would be a bug in the debugger, not a
> > fundamental problem with the approach.
>
> Agreed. We can probably minify by default. If we *really* need
> un-minified for debugging, we can perhaps just hack the source. Or
> introduce a setting for skipping minification.
>
> > For JavaScript: I guess the idea is to copy e.g. bootstrap.min.js from
> > node_modules to the public directory instead of shipping it ourselves, and
> > similar for other libraries.
> > Also this would just be another step taken by setup-app, right?
> > Do you see more complexity?
>
> Yes, I think JavaScript (and various other support files from npm
> packages, like css and images) should be handled exactly as the npm css
> generation.
>
> >> I don't think I would like to have the application run this
> >> automatically at run-time, but could it be feasible to let the
> >> application fail to start if front-end code hasn't been generated or is
> >> outdated? (That kind of goes in the opposite direction of allowing it to
> >> be hosted elsewhere ... but that could be a special case.)
> > Failing to start when the front-end code is not generated at all seems feasible
> > without much issue: basically we can check the presence of the files referred to
> > in the root.html/base.html.
> > Checking whether it's up to date is more complex. Checking that the kallithea
> > .css is generated from the current .less file may be achievable. For example by
> > saving the md5sum of the .less file into the .css (I didn't check yet if that is
> > possible via less) and checking that when starting the application.
> > But to check whether all other dependencies are fine may be more complex. As we
> > don't yet have such cases I find it hard to reason about it.
>
> Some basic time stamp comparison on main files can probably cover 90% of
> the problems and help us catch a lot of problems.
>
>
> Interesting discussion that helps understanding concerns and getting
> alignment.
>
> In light of this, will you make another iteration of your proposal?

I agree that the discussion is interesting and helps exploring which
way we should go.
However, in practical terms it is still too broad/vague to me as to
what to actually implement now.
I would like to take a first step in the right direction that covers
our current needs, which would allow making a release, and postpone
other improvements to a later moment. For example, the
rescoping/moving of 'kallithea/public'  may not be required yet.
Could you list which specific changes you think we should make to the
current proposal?

Thanks,
Thomas