PR#1786: Fix alembic/env.py and add support for createdb.py --initial - pagure

		`@@ -102,10 +102,17 @@`

		`mkdir -p lcl/{repos,docs,forks,tickets,requests,remotes,attachments,releases}`

		+ * Copy and edit the alembic.ini file (especially the ``script_location`` key)::
		`+`
		`+ cp files/alembic.ini .`
		`+ vim alembic.ini`
		`+`
		+ * Set the ``script_location`` to ``alembic``, ie: the folder where the revisions
		+ are stored, relative to the location of the ``alembic.ini`` file.

		`* Create the inital database scheme::`

		`- python createdb.py`
		`+ python createdb.py --initial alembic.ini`

		`* Start a worker, in one terminal::`

createdb.py

file modified

+42 -3

		`@@ -1,14 +1,53 @@`
		`#!/usr/bin/env python2`

		`+ from __future__ import print_function`
		`+`
		`# These two lines are needed to run on EL6`
		`__requires__ = ['SQLAlchemy >= 0.8', 'jinja2 >= 2.4']`
		`- import pkg_resources`
		`+ import pkg_resources # noqa`
		`+`
		`+ import argparse`
		`+ import sys`
		`+ import os`
		`+`
		`+`
		`+ parser = argparse.ArgumentParser(`
		`+ description='Create/Update the Pagure database')`
		`+ parser.add_argument(`
		`+ '--config', '-c', dest='config',`
		`+ help='Configuration file to use for pagure.')`
		`+ parser.add_argument(`
		`+ '--initial', '-i', dest='alembic_cfg',`
		`+ help='With this option, the database will be automatically stamped to '`
		`+ 'the latest version according to alembic. Point to the alembic.ini '`
		`+ 'file to use.')`
		`+`
		`+`
		`+ args = parser.parse_args()`
		`+`
		`+ if args.config:`
		`+ config = args.config`
		`+ if not config.startswith('/'):`
		`+ here = os.path.join(os.path.dirname(os.path.abspath(__file__)))`
		`+ config = os.path.join(here, config)`
		`+ os.environ['PAGURE_CONFIG'] = config`
		`+`
		`+`
		`+ if args.alembic_cfg:`
		`+ if not args.alembic_cfg.endswith('alembic.ini'):`
		`+ print('--initial should point to the alembic.ini file to use.')`
		`+ sys.exit(1)`
		`+ if not os.path.exists(args.alembic_cfg):`
		+ print('The file `{0}` could not be found'.format(args.alembic_cfg))
		`+ sys.exit(2)`
		`+`

		`from pagure import APP`
		`from pagure.lib import model`

		`model.create_tables(`
		`APP.config['DB_URL'],`
		`- APP.config.get('PATH_ALEMBIC_INI', None),`
		`+ APP.config.get('PATH_ALEMBIC_INI', args.alembic_cfg),`
		`acls=APP.config.get('ACLS', {}),`
		`- debug=True)`
		`+ debug=True`
		`+ )`

doc/install.rst

file modified

+28 -23

		`@@ -218,51 +218,56 @@`
		`to the database, and create the database itself, you can now create the`
		`tables, the database scheme.`

		+ For changes to existing tables, we rely on `Alembic <http://alembic.readthedocs.org/>`_.
		+ It uses `revisions` to perform the upgrades, but to know which upgrades are
		`+ needed and which are already done, the current revision needs to be saved`
		`+ in the database. This will allow alembic to know and apply the new revision`
		`+ when running it.`
		`+`
		+ In the ``alembic.ini`` file, one of the configuration key is most important:
		+ ``script_location`` which is the path to the ``versions`` folder containing
		+ all the alembic migration files. The ``sqlalchemy.url`` configuration key if
		`+ missing will be replaced by the url filled in the configuration file of`
		`+ pagure.`
		`+`
		`To create the database tables, you need to run the script`
		``/usr/share/pagure/pagure_createdb.py`` and specify the configuration
		`- file to use via an environment variable.`
		`+ to use for pagure and for alembic.`

		`For example:`
		`::`

		`- PAGURE_CONFIG=/etc/pagure/pagure.cfg python /usr/share/pagure/pagure_createdb.py`
		`+ python /usr/share/pagure/pagure_createdb.py -c /etc/pagure/pagure.cfg -i /etc/pagure/alembic.ini`

		This will tell ``/usr/share/pagure/pagure_createdb.py`` to use the database
		- information specified in the file ``/etc/pagure/pagure.cfg``.
		+ information specified in the file ``/etc/pagure/pagure.cfg`` and to stamp
		`+ the database at the last alembic revision.`

		`.. warning:: Pagure's default configuration is using sqlite. This is fine`
		`for development purpose but not for production use as sqlite does`
		`not support all the operations needed when updating the database`
		`schema. Do use PostgreSQL, MySQL or MariaDB in production.`

		`- * Stamp the alembic revision`
		`-`
		For changes to existing tables, we rely on `Alembic <http://alembic.readthedocs.org/>`_.
		It uses `revisions` to perform the upgrades, but to know which upgrades are
		`needed and which are already done, the current revision needs to be saved`
		`in the database. This will allow alembic to know apply the new revision when`
		`running it.`

		`- You can save the current revision in the database using the following command:`
		`- ::`
		`-`
		`- cd /etc/pagure`
		`- alembic stamp $(alembic heads \| awk '{ print $1 }')`
		`-`
		- The ``cd /etc/pagure`` is needed as the command must be run in the folder
		- where the file ``alembic.ini`` is. This file contains two important pieces
		`- of information:`
		`-`
		- * ``sqlalchemy.url`` which is the URL used to connect to the database, likely
		- the same URL as the one in ``pagure.cfg``.
		+ In the ``alembic.ini`` file, one of the configuration key is most important:
		+ ``script_location`` which is the path to the ``versions`` folder containing
		+ all the alembic migration files. The ``sqlalchemy.url`` configuration key if
		`+ missing will be replaced by the url filled in the configuration file of`
		`+ pagure.`

		- * ``script_location`` which is the path to the ``versions`` folder containing
		`- all the alembic migration files.`
		+ .. warning:: Calling ``pagure_createdb.py`` is asked regularly in the
		`+ UPGRADING.rst documentation, especially to handle database schema`
		+ changes upon upgrades, but the ``--initial`` argument should only
		`+ be used the first time as it will otherwise break upgrading the`
		`+ database schema via alembic.`

		- The ``alembic stamp`` command is the one actually saving the current revision
		- into the database. This current revision is found using ``alembic heads``
		`- which returns the most recent revision found by alembic, and since the`
		`- database was just created, it is at the latest revision.`
		+ .. note:: When install from source the script is called ``createdb.py`` and
		+ not ``pagure_createdb.py``.


		`Set up virus scanning`

pingou commented 7 years ago

Looks like something went wrong when merging https://pagure.io/pagure/pull-request/1738
and this didn't get merged correctly so here it is.

cep commented on line 26 of doc/install.rst 7 years ago

It would be nice to introduce this section before we talk about alembic config files. i.e. Move this paragraph to before we start using alembic.

cep commented 7 years ago

Rest looks good :) :thumbsup:

vivekanand1101 commented 7 years ago

:thumbsup:

pingou commented on line 26 of doc/install.rst 7 years ago

Good call

1 new commit added

Introduce alembic before it is used by createdb

7 years ago

rebased

7 years ago

pingou commented 7 years ago

@cep want to have a final look?

cep commented on line 8 of doc/install.rst 7 years ago

alembic to know and apply

cep commented on line 63 of createdb.py 7 years ago

I'm not completely sure, but it looks like the database URL is replaced by the one in pagure's config, irrespective of whether it is present in alembic.ini. Whereas, the docs say it is only replaced when missing.

Is that right, or did I miss something?

cep commented on line 26 of doc/install.rst 7 years ago

Looks like this is just createdb.py (not pagure_createdb.py) now?

pingou commented on line 8 of doc/install.rst 7 years ago

indeed :)

pingou commented on line 26 of doc/install.rst 7 years ago

It is installed as pagure_createdb.py by the spec file, I do see the problem for user who installed outside of the RPM. I guess this might deserve a .. note::

pingou commented on line 63 of createdb.py 7 years ago

You're reading this right because the doc is referring to the changes in env.py rather than the changes here.
Do you think we should we try to get the url and only replace it if it's missing?

cep commented on line 26 of doc/install.rst 7 years ago

Yes, if you cloned the source, then it's just createdb.py. +1 for adding a note :)

cep commented on line 63 of createdb.py 7 years ago

That would work! :)

cep commented 7 years ago

@pingou Sorry, I don't know how I missed the notification about your comments. This should be good to go after the couple of minor changes mentioned :)

rebased

7 years ago

pingou commented 7 years ago

Jenkins is failing due to faitout :(

rebased

6 years ago