From e307d74092ec81db29c06094d3e18e040f906e27 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 08 2017 08:59:21 +0000 Subject: [PATCH 1/8] Improve the createdb script to support stamping the database in the initial run This commit adds an --initial argument that should be pointed to the alembic.ini file to use. Using this information, the database will be created and stamped to the latest revision present in alembic. Fixes https://pagure.io/pagure/issue/1696 --- diff --git a/createdb.py b/createdb.py index eb78c78..ccec69b 100644 --- a/createdb.py +++ b/createdb.py @@ -3,12 +3,61 @@ # These two lines are needed to run on EL6 __requires__ = ['SQLAlchemy >= 0.8', 'jinja2 >= 2.4'] import pkg_resources +import sys + +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), acls=APP.config.get('ACLS', {}), debug=True) + + +if args.alembic_cfg: + from alembic import command + from alembic.config import Config + + alembic_cfg = Config(args.alembic_cfg) + alembic_cfg.set_main_option("url", APP.config['DB_URL']) + command.current(alembic_cfg) + command.stamp(alembic_cfg, "head") + command.current(alembic_cfg) From f92b8c772c38fe48fe2ec24965fc8113ba4e346e Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 08 2017 08:59:21 +0000 Subject: [PATCH 2/8] Adjust the documentation around createdb.py to mention --initial --- diff --git a/doc/install.rst b/doc/install.rst index 0001d9f..0fd9d5e 100644 --- a/doc/install.rst +++ b/doc/install.rst @@ -220,49 +220,38 @@ tables, the database scheme. 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 `_. 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``. - -* ``script_location`` which is the path to the ``versions`` folder containing - all the alembic migration files. +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. -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. +.. warning:: Calling ``pagure_createdb.py`` is asked regularly, 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. Set up virus scanning From 3335c026a6a32d7be2187236267bb460ec9bc392 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 08 2017 08:59:21 +0000 Subject: [PATCH 3/8] Adjust the README to document --initial --- diff --git a/README.rst b/README.rst index 71aa328..15eaa3e 100644 --- a/README.rst +++ b/README.rst @@ -102,10 +102,17 @@ Manually 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 are stored + the revisions relative to where the ``alembic.ini`` file is. * Create the inital database scheme:: - python createdb.py + python createdb.py --initial alembic.ini * Start a worker, in one terminal:: From 5d2da2a6a3230ebf7cfc3cb448e4d91e8069fc68 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 08 2017 08:59:21 +0000 Subject: [PATCH 4/8] Reword the instructions in the README as advised by @cep in the review --- diff --git a/README.rst b/README.rst index 15eaa3e..5a43be4 100644 --- a/README.rst +++ b/README.rst @@ -107,8 +107,8 @@ Manually cp files/alembic.ini . vim alembic.ini -* Set the ``script_location`` to ``alembic``, ie: the folder where are stored - the revisions relative to where the ``alembic.ini`` file is. +* 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:: From a9064af1c5b98806e1af46a47b7cd846713c462c Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 08 2017 08:59:21 +0000 Subject: [PATCH 5/8] Inform the user of what is being printed upon createdb.py --initial --- diff --git a/createdb.py b/createdb.py index ccec69b..6ab0727 100644 --- a/createdb.py +++ b/createdb.py @@ -1,5 +1,7 @@ #!/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 @@ -58,6 +60,6 @@ if args.alembic_cfg: alembic_cfg = Config(args.alembic_cfg) alembic_cfg.set_main_option("url", APP.config['DB_URL']) - command.current(alembic_cfg) command.stamp(alembic_cfg, "head") + print('Current alembic revision id is at:') command.current(alembic_cfg) From f2139675f944f87b82d5e46a934a16979b48a374 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 08 2017 08:59:21 +0000 Subject: [PATCH 6/8] Introduce alembic before it is used by createdb --- diff --git a/doc/install.rst b/doc/install.rst index 0fd9d5e..5d3d9a9 100644 --- a/doc/install.rst +++ b/doc/install.rst @@ -218,6 +218,18 @@ Once you have specified in the configuration file the to url used to connect 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 `_. +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. + +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 to use for pagure and for alembic. @@ -248,10 +260,11 @@ 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. -.. warning:: Calling ``pagure_createdb.py`` is asked regularly, 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. +.. 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. Set up virus scanning From 89bddf0c64d67b0e1004f9634e66b3de4e4775f9 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 08 2017 08:59:21 +0000 Subject: [PATCH 7/8] model.create_tables() can already stamp the database so rely on this --- diff --git a/createdb.py b/createdb.py index 6ab0727..d0ac61f 100644 --- a/createdb.py +++ b/createdb.py @@ -4,8 +4,7 @@ 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 sys +import pkg_resources # noqa import argparse import sys @@ -46,20 +45,9 @@ if args.alembic_cfg: 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) - - -if args.alembic_cfg: - from alembic import command - from alembic.config import Config - - alembic_cfg = Config(args.alembic_cfg) - alembic_cfg.set_main_option("url", APP.config['DB_URL']) - command.stamp(alembic_cfg, "head") - print('Current alembic revision id is at:') - command.current(alembic_cfg) + debug=True +) From 525e441b399ee50cd8477dad8735401fb4f4a3b2 Mon Sep 17 00:00:00 2001 From: Pierre-Yves Chibon Date: Dec 08 2017 08:59:21 +0000 Subject: [PATCH 8/8] Adjust the documentation around the use of createdb.py --- diff --git a/doc/install.rst b/doc/install.rst index 5d3d9a9..a5c453a 100644 --- a/doc/install.rst +++ b/doc/install.rst @@ -221,8 +221,8 @@ tables, the database scheme. For changes to existing tables, we rely on `Alembic `_. 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. +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 @@ -266,6 +266,9 @@ pagure. be used the first time as it will otherwise break upgrading the database schema via alembic. +.. note:: When install from source the script is called ``createdb.py`` and + not ``pagure_createdb.py``. + Set up virus scanning ---------------------