From 907b45244593dfbb07bb9fdbbe5c249b55085c7b Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 1 Jun 2017 16:02:48 -0700 Subject: [PATCH 01/10] Initialize alembic --- nbgrader/alembic.ini | 68 ++++++++++++++++++++++++++++++++ nbgrader/alembic/README | 1 + nbgrader/alembic/env.py | 70 +++++++++++++++++++++++++++++++++ nbgrader/alembic/script.py.mako | 24 +++++++++++ 4 files changed, 163 insertions(+) create mode 100644 nbgrader/alembic.ini create mode 100644 nbgrader/alembic/README create mode 100644 nbgrader/alembic/env.py create mode 100644 nbgrader/alembic/script.py.mako diff --git a/nbgrader/alembic.ini b/nbgrader/alembic.ini new file mode 100644 index 000000000..4a9cdb671 --- /dev/null +++ b/nbgrader/alembic.ini @@ -0,0 +1,68 @@ +# A generic, single database configuration. + +[alembic] +# path to migration scripts +script_location = alembic + +# template used to generate migration files +# file_template = %%(rev)s_%%(slug)s + +# max length of characters to apply to the +# "slug" field +#truncate_slug_length = 40 + +# set to 'true' to run the environment during +# the 'revision' command, regardless of autogenerate +# revision_environment = false + +# set to 'true' to allow .pyc and .pyo files without +# a source .py file to be detected as revisions in the +# versions/ directory +# sourceless = false + +# version location specification; this defaults +# to alembic/versions. When using multiple version +# directories, initial revisions must be specified with --version-path +# version_locations = %(here)s/bar %(here)s/bat alembic/versions + +# the output encoding used when revision files +# are written from script.py.mako +# output_encoding = utf-8 + +sqlalchemy.url = driver://user:pass@localhost/dbname + + +# Logging configuration +[loggers] +keys = root,sqlalchemy,alembic + +[handlers] +keys = console + +[formatters] +keys = generic + +[logger_root] +level = WARN +handlers = console +qualname = + +[logger_sqlalchemy] +level = WARN +handlers = +qualname = sqlalchemy.engine + +[logger_alembic] +level = INFO +handlers = +qualname = alembic + +[handler_console] +class = StreamHandler +args = (sys.stderr,) +level = NOTSET +formatter = generic + +[formatter_generic] +format = %(levelname)-5.5s [%(name)s] %(message)s +datefmt = %H:%M:%S diff --git a/nbgrader/alembic/README b/nbgrader/alembic/README new file mode 100644 index 000000000..98e4f9c44 --- /dev/null +++ b/nbgrader/alembic/README @@ -0,0 +1 @@ +Generic single-database configuration. \ No newline at end of file diff --git a/nbgrader/alembic/env.py b/nbgrader/alembic/env.py new file mode 100644 index 000000000..058378b9d --- /dev/null +++ b/nbgrader/alembic/env.py @@ -0,0 +1,70 @@ +from __future__ import with_statement +from alembic import context +from sqlalchemy import engine_from_config, pool +from logging.config import fileConfig + +# this is the Alembic Config object, which provides +# access to the values within the .ini file in use. +config = context.config + +# Interpret the config file for Python logging. +# This line sets up loggers basically. +fileConfig(config.config_file_name) + +# add your model's MetaData object here +# for 'autogenerate' support +# from myapp import mymodel +# target_metadata = mymodel.Base.metadata +target_metadata = None + +# other values from the config, defined by the needs of env.py, +# can be acquired: +# my_important_option = config.get_main_option("my_important_option") +# ... etc. + + +def run_migrations_offline(): + """Run migrations in 'offline' mode. + + This configures the context with just a URL + and not an Engine, though an Engine is acceptable + here as well. By skipping the Engine creation + we don't even need a DBAPI to be available. + + Calls to context.execute() here emit the given string to the + script output. + + """ + url = config.get_main_option("sqlalchemy.url") + context.configure( + url=url, target_metadata=target_metadata, literal_binds=True) + + with context.begin_transaction(): + context.run_migrations() + + +def run_migrations_online(): + """Run migrations in 'online' mode. + + In this scenario we need to create an Engine + and associate a connection with the context. + + """ + connectable = engine_from_config( + config.get_section(config.config_ini_section), + prefix='sqlalchemy.', + poolclass=pool.NullPool) + + with connectable.connect() as connection: + context.configure( + connection=connection, + target_metadata=target_metadata + ) + + with context.begin_transaction(): + context.run_migrations() + +if context.is_offline_mode(): + run_migrations_offline() +else: + run_migrations_online() diff --git a/nbgrader/alembic/script.py.mako b/nbgrader/alembic/script.py.mako new file mode 100644 index 000000000..2c0156303 --- /dev/null +++ b/nbgrader/alembic/script.py.mako @@ -0,0 +1,24 @@ +"""${message} + +Revision ID: ${up_revision} +Revises: ${down_revision | comma,n} +Create Date: ${create_date} + +""" +from alembic import op +import sqlalchemy as sa +${imports if imports else ""} + +# revision identifiers, used by Alembic. +revision = ${repr(up_revision)} +down_revision = ${repr(down_revision)} +branch_labels = ${repr(branch_labels)} +depends_on = ${repr(depends_on)} + + +def upgrade(): + ${upgrades if upgrades else "pass"} + + +def downgrade(): + ${downgrades if downgrades else "pass"} From eb8b99347b0322c86b43b38eb98e6bc6f8656171 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 1 Jun 2017 16:03:15 -0700 Subject: [PATCH 02/10] Add alembic to requirements --- setup.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/setup.py b/setup.py index f8532116d..0f16efb00 100644 --- a/setup.py +++ b/setup.py @@ -40,6 +40,15 @@ for filename in filenames: static_files.append(os.path.join(root, filename)) +# get paths to all the alembic files +alembic_files = ["alembic.ini"] +for (dirname, dirnames, filenames) in os.walk("nbgrader/alembic"): + root = os.path.relpath(dirname, "nbgrader") + for filename in filenames: + if filename.endswith(".pyc"): + continue + alembic_files.append(os.path.join(root, filename)) + name = 'nbgrader' here = os.path.abspath(os.path.dirname(__file__)) @@ -67,7 +76,7 @@ ], packages=find_packages(), package_data={ - 'nbgrader': extension_files + docs_files, + 'nbgrader': extension_files + docs_files + alembic_files, 'nbgrader.nbgraderformat': ["*.json"], 'nbgrader.server_extensions.formgrader': static_files, 'nbgrader.tests': [ @@ -93,6 +102,7 @@ "six>=1.9", "requests", "jsonschema", + "alembic" ] ) From 4062cfabeb4662ea354061e90e1a1967582b0870 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 1 Jun 2017 16:30:34 -0700 Subject: [PATCH 03/10] Add functionality to upgrade database --- nbgrader/alembic.ini | 5 +- nbgrader/alembic/README | 2 +- .../b6d005d67074_initial_alembic_revision.py | 24 ++++++ nbgrader/apps/dbapp.py | 43 ++++++++++ nbgrader/dbutil.py | 81 +++++++++++++++++++ 5 files changed, 151 insertions(+), 4 deletions(-) create mode 100644 nbgrader/alembic/versions/b6d005d67074_initial_alembic_revision.py create mode 100644 nbgrader/dbutil.py diff --git a/nbgrader/alembic.ini b/nbgrader/alembic.ini index 4a9cdb671..fca4c335e 100644 --- a/nbgrader/alembic.ini +++ b/nbgrader/alembic.ini @@ -2,7 +2,8 @@ [alembic] # path to migration scripts -script_location = alembic +script_location = {alembic_dir} +sqlalchemy.url = {db_url} # template used to generate migration files # file_template = %%(rev)s_%%(slug)s @@ -29,8 +30,6 @@ script_location = alembic # are written from script.py.mako # output_encoding = utf-8 -sqlalchemy.url = driver://user:pass@localhost/dbname - # Logging configuration [loggers] diff --git a/nbgrader/alembic/README b/nbgrader/alembic/README index 98e4f9c44..8ddaddeac 100644 --- a/nbgrader/alembic/README +++ b/nbgrader/alembic/README @@ -1 +1 @@ -Generic single-database configuration. \ No newline at end of file +This is the alembic configuration for nbgrader database migrations. \ No newline at end of file diff --git a/nbgrader/alembic/versions/b6d005d67074_initial_alembic_revision.py b/nbgrader/alembic/versions/b6d005d67074_initial_alembic_revision.py new file mode 100644 index 000000000..672c0435e --- /dev/null +++ b/nbgrader/alembic/versions/b6d005d67074_initial_alembic_revision.py @@ -0,0 +1,24 @@ +"""Initial alembic revision + +Revision ID: b6d005d67074 +Revises: +Create Date: 2017-06-01 16:05:35.868678 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'b6d005d67074' +down_revision = None +branch_labels = None +depends_on = None + + +def upgrade(): + pass + + +def downgrade(): + pass diff --git a/nbgrader/apps/dbapp.py b/nbgrader/apps/dbapp.py index 4463662c5..3170d5db0 100644 --- a/nbgrader/apps/dbapp.py +++ b/nbgrader/apps/dbapp.py @@ -3,12 +3,15 @@ import csv import os +import shutil from textwrap import dedent from traitlets import default, Unicode, Bool +from datetime import datetime from . import NbGrader from ..api import Gradebook, MissingEntry +from .. import dbutil aliases = { 'log-level': 'Application.log_level', @@ -382,6 +385,38 @@ def start(self): super(DbAssignmentApp, self).start() +class DbUpgradeApp(NbGrader): + """Based on the `jupyterhub upgrade-db` command found in jupyterhub.app.UpgradeDB""" + + name = 'nbgrader-db-upgrade' + description = u'Upgrade the database schema to the latest version' + + def _backup_db_file(self, db_file): + """Backup a database file""" + if not os.path.exists(db_file): + return + + timestamp = datetime.now().strftime('.%Y-%m-%d-%H%M%S') + backup_db_file = db_file + timestamp + for i in range(1, 10): + if not os.path.exists(backup_db_file): + break + backup_db_file = '{}.{}.{}'.format(db_file, timestamp, i) + if os.path.exists(backup_db_file): + self.fail("backup db file already exists: %s" % backup_db_file) + + self.log.info("Backing up %s => %s", db_file, backup_db_file) + shutil.copy(db_file, backup_db_file) + + def start(self): + super(DbUpgradeApp, self).start() + if (self.coursedir.db_url.startswith('sqlite:///')): + db_file = self.coursedir.db_url.split(':///', 1)[1] + self._backup_db_file(db_file) + self.log.info("Upgrading %s", self.coursedir.db_url) + dbutil.upgrade(self.coursedir.db_url) + + class DbApp(NbGrader): name = 'nbgrader-db' @@ -404,6 +439,14 @@ class DbApp(NbGrader): """ ).strip() ), + upgrade=( + DbUpgradeApp, + dedent( + """ + Upgrade database schema to latest version. + """ + ).strip() + ), ) @default("classes") diff --git a/nbgrader/dbutil.py b/nbgrader/dbutil.py new file mode 100644 index 000000000..ae61994e8 --- /dev/null +++ b/nbgrader/dbutil.py @@ -0,0 +1,81 @@ +"""Database utilities for nbgrader""" +# Based on jupyterhub.dbutil + +import os +import sys + +from contextlib import contextmanager +from subprocess import check_call +from tempfile import TemporaryDirectory + +_here = os.path.abspath(os.path.dirname(__file__)) + +ALEMBIC_INI_TEMPLATE_PATH = os.path.join(_here, 'alembic.ini') +ALEMBIC_DIR = os.path.join(_here, 'alembic') + + +def write_alembic_ini(alembic_ini='alembic.ini', db_url='sqlite:///gradebook.db'): + """Write a complete alembic.ini from our template. + Parameters + ---------- + alembic_ini: str + path to the alembic.ini file that should be written. + db_url: str + The SQLAlchemy database url, e.g. `sqlite:///gradebook.db`. + """ + with open(ALEMBIC_INI_TEMPLATE_PATH) as f: + alembic_ini_tpl = f.read() + + with open(alembic_ini, 'w') as f: + f.write( + alembic_ini_tpl.format( + alembic_dir=ALEMBIC_DIR, + db_url=db_url, + ) + ) + + +@contextmanager +def _temp_alembic_ini(db_url): + """Context manager for temporary JupyterHub alembic directory + Temporarily write an alembic.ini file for use with alembic migration scripts. + Context manager yields alembic.ini path. + Parameters + ---------- + db_url: str + The SQLAlchemy database url, e.g. `sqlite:///gradebook.db`. + Returns + ------- + alembic_ini: str + The path to the temporary alembic.ini that we have created. + This file will be cleaned up on exit from the context manager. + """ + with TemporaryDirectory() as td: + alembic_ini = os.path.join(td, 'alembic.ini') + write_alembic_ini(alembic_ini, db_url) + yield alembic_ini + + +def upgrade(db_url, revision='head'): + """Upgrade the given database to revision. + db_url: str + The SQLAlchemy database url, e.g. `sqlite:///gradebook.db`. + revision: str [default: head] + The alembic revision to upgrade to. + """ + with _temp_alembic_ini(db_url) as alembic_ini: + check_call( + ['alembic', '-c', alembic_ini, 'upgrade', revision] + ) + + +def _alembic(*args): + """Run an alembic command with a temporary alembic.ini""" + with _temp_alembic_ini('sqlite:///gradebook.db') as alembic_ini: + check_call( + ['alembic', '-c', alembic_ini] + list(args) + ) + + +if __name__ == '__main__': + _alembic(*sys.argv[1:]) From d709a553d73438a88e56df2aa96532f0c4d570e9 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 1 Jun 2017 17:08:47 -0700 Subject: [PATCH 04/10] Upgrade database to add kernelspec column --- .../versions/50a4d84c131a_add_kernelspecs.py | 26 +++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 nbgrader/alembic/versions/50a4d84c131a_add_kernelspecs.py diff --git a/nbgrader/alembic/versions/50a4d84c131a_add_kernelspecs.py b/nbgrader/alembic/versions/50a4d84c131a_add_kernelspecs.py new file mode 100644 index 000000000..abe268665 --- /dev/null +++ b/nbgrader/alembic/versions/50a4d84c131a_add_kernelspecs.py @@ -0,0 +1,26 @@ +"""add kernelspecs + +Revision ID: 50a4d84c131a +Revises: b6d005d67074 +Create Date: 2017-06-01 16:48:02.243764 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = '50a4d84c131a' +down_revision = 'b6d005d67074' +branch_labels = None +depends_on = None + + +def upgrade(): + op.add_column('notebook', sa.Column( + 'kernelspec', sa.String(1024), nullable=False, + server_default='{"display_name": "Python", "name": "python", "language": "python"}')) + + +def downgrade(): + op.drop_column('notebook', 'kernelspec') From ff3e19a035931da1930fff67aaad209c5dfe3240 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 1 Jun 2017 17:09:01 -0700 Subject: [PATCH 05/10] Print out information about upgrading database --- nbgrader/apps/baseapp.py | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/nbgrader/apps/baseapp.py b/nbgrader/apps/baseapp.py index ad6278e18..80b567ad8 100644 --- a/nbgrader/apps/baseapp.py +++ b/nbgrader/apps/baseapp.py @@ -10,6 +10,7 @@ import traceback import logging import shutil +import sqlalchemy from jupyter_core.application import JupyterApp from nbconvert.exporters.export import exporter_map @@ -580,6 +581,16 @@ def _handle_failure(gd): errors.append((gd['assignment_id'], gd['student_id'])) _handle_failure(gd) + except sqlalchemy.exc.OperationalError: + _handle_failure(gd) + self.log.error(traceback.format_exc()) + self.fail( + "There was an error accessing the nbgrader database. This " + "may occur if you recently upgraded nbgrader. To resolve " + "the issue, first BACK UP your database and then run the " + "command `nbgrader db upgrade`." + ) + except Exception: self.log.error("There was an error processing assignment: %s", assignment) self.log.error(traceback.format_exc()) From c875ee8b5b41a9751c9287cf74d9d619c3ae27a8 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 1 Jun 2017 17:14:15 -0700 Subject: [PATCH 06/10] Add test for upgrading database --- nbgrader/tests/apps/files/gradebook.db | Bin 0 -> 35840 bytes nbgrader/tests/apps/test_nbgrader_db.py | 19 +++++++++++++++++++ 2 files changed, 19 insertions(+) create mode 100644 nbgrader/tests/apps/files/gradebook.db diff --git a/nbgrader/tests/apps/files/gradebook.db b/nbgrader/tests/apps/files/gradebook.db new file mode 100644 index 0000000000000000000000000000000000000000..a9a3ebbb4f7fadc1fc38790e68837f57c9d5367d GIT binary patch literal 35840 zcmeI5O>Y}T7{~X;v6F4x_jc2!flYm|S8`NrD^e>!;F@h}HFc=#0u_hV*4_jw_HK#S z^hGKt3GH{_#+4g44txQ6;efsw=%C&MmS1QUoPeYH$o)j)Zi!( z2oXs0ub32biQ&)j&p4L*jr*S2VJ=P9<{X&*WcP70C9bTnPZnL7#SjV1Fx^Uu7GMmi zHK4{ARHtC2he~T$YoJOE-FL)1a*5^eoiz^AbLy zbCj#=b)~vmvz0J8YEN0JD_51eQmZJpC@GF7mmw<>ZIZP-v!k#PWYGT*e5YIggT!P7 zFZetf2xjfUAkA-}{Le7F!3q2dpI|cVuk5${2LCJ=Ds~?ob`)Yy-g7c^b6acaYE$oY zaOo(Ojo3aR2SZXQNd?8B?p!+Tg@HXtuEOiRCOnpVe=kg#v=;Vaja%5^D+`%|q+xop#H>Dj!L?P}Xw zE%UySl|xxg9)rwI-|nSn>BuBY&!xo0MRu?3gxT+h|9+SX zv2XoX0VWbJJOqy$JMHr(PmF#j$RWN|DOMIoN#}f5)uLZIa#4s>q2O0Y z^FO%Rc3|>Pl$=#sN4jz>DONM1oamYT?WUWxXbiv~PQw5Hf^iNCKp=JmVE!Mwkb}cO zARqwwAE1B&5QrTCxc`q`$iZPC5D2*i$negB_eE->6X+zk6OD*S?e8RySqu_vEMiG>3DgO)1xGU0(++G3PF; zhZfW`4fURGXdUaJt0pqaXV72OE^Z!)qHzU$&(!V-r9o0xAIa! zMyunk9FHmjD-t5GQ~T78?Uj$F#7u^LT6ND1EGr`#KS7@hc%KO;!FX*b)?50k4-V)* zJ7#kWp9++A)a{(qP(EqcXF&5ay}8xvcY_nnS9<&4hn#7X#fe1aK?x}VXCE3;FZ!N; zIQX_JuYEcRGR*&DH{^rEKp-Fh_x}M3C;)-j5rF)UUC6;)4uSt zV)hF9s^7Rjuj^_?U+aX-lfpQ5`G23$ zflRJRzC^#@#DhC zqX$p3`<*7g1j4t?S5CfWBg7X#_>G7#|BpD#!EQic3JJjd{}culTmk|S2|)fw1O>YR zfhi;a_y1EERB#ChL?i&$|A?SqHy|*D1R(#XFsR@X5Qs>+R_d6dBSY`&-{}IC& Kn1MhH2>b&?oziOn literal 0 HcmV?d00001 diff --git a/nbgrader/tests/apps/test_nbgrader_db.py b/nbgrader/tests/apps/test_nbgrader_db.py index 01939fc30..4918feee5 100644 --- a/nbgrader/tests/apps/test_nbgrader_db.py +++ b/nbgrader/tests/apps/test_nbgrader_db.py @@ -1,5 +1,7 @@ import pytest import datetime +import shutil +import os from textwrap import dedent from os.path import join @@ -282,3 +284,20 @@ def test_assignment_import(self, db, temp_cwd): assert assignment.duedate == datetime.datetime(2017, 1, 8, 16, 31, 22) assignment = gb.find_assignment("bar") assert assignment.duedate is None + + def test_upgrade_db(self, course_dir): + # add assignment files + self._copy_file(join("files", "test.ipynb"), join(course_dir, "source", "ps1", "p1.ipynb")) + self._copy_file(join("files", "test.ipynb"), join(course_dir, "source", "ps1", "p2.ipynb")) + + # replace the gradebook with an old version + self._copy_file(join("files", "gradebook.db"), join(course_dir, "gradebook.db")) + + # check that nbgrader assign fails + run_nbgrader(["assign", "ps1", "--create"], retcode=1) + + # upgrade the database + run_nbgrader(["db", "upgrade"]) + + # check that nbgrader assign passes + run_nbgrader(["assign", "ps1", "--create"]) From 7576c126aa39fd2f452e34e0355c915570e5f4f3 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 1 Jun 2017 17:22:36 -0700 Subject: [PATCH 07/10] Don't require that python is the default kernelspec --- nbgrader/alembic/versions/50a4d84c131a_add_kernelspecs.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nbgrader/alembic/versions/50a4d84c131a_add_kernelspecs.py b/nbgrader/alembic/versions/50a4d84c131a_add_kernelspecs.py index abe268665..d32d47e13 100644 --- a/nbgrader/alembic/versions/50a4d84c131a_add_kernelspecs.py +++ b/nbgrader/alembic/versions/50a4d84c131a_add_kernelspecs.py @@ -19,7 +19,7 @@ def upgrade(): op.add_column('notebook', sa.Column( 'kernelspec', sa.String(1024), nullable=False, - server_default='{"display_name": "Python", "name": "python", "language": "python"}')) + server_default='{}')) def downgrade(): From 4f14dd7a9d92ee6dbf7151631e4a1466dc4af77d Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 1 Jun 2017 17:39:33 -0700 Subject: [PATCH 08/10] Fix python 2 bug with TemporaryDirectory --- nbgrader/dbutil.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/nbgrader/dbutil.py b/nbgrader/dbutil.py index ae61994e8..0beb9ac13 100644 --- a/nbgrader/dbutil.py +++ b/nbgrader/dbutil.py @@ -3,10 +3,11 @@ import os import sys +import tempfile +import shutil from contextlib import contextmanager from subprocess import check_call -from tempfile import TemporaryDirectory _here = os.path.abspath(os.path.dirname(__file__)) @@ -50,10 +51,13 @@ def _temp_alembic_ini(db_url): The path to the temporary alembic.ini that we have created. This file will be cleaned up on exit from the context manager. """ - with TemporaryDirectory() as td: + td = tempfile.mkdtemp() + try: alembic_ini = os.path.join(td, 'alembic.ini') write_alembic_ini(alembic_ini, db_url) yield alembic_ini + finally: + shutil.rmtree(td) def upgrade(db_url, revision='head'): From 61efbfa4c1a9547eb46e74ae3f4c2652b29659a4 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 1 Jun 2017 17:46:52 -0700 Subject: [PATCH 09/10] Add developer documentation fo modifying the database --- .../source/contributor_guide/database.rst | 39 +++++++++++++++++++ nbgrader/docs/source/index.rst | 1 + 2 files changed, 40 insertions(+) create mode 100644 nbgrader/docs/source/contributor_guide/database.rst diff --git a/nbgrader/docs/source/contributor_guide/database.rst b/nbgrader/docs/source/contributor_guide/database.rst new file mode 100644 index 000000000..417782739 --- /dev/null +++ b/nbgrader/docs/source/contributor_guide/database.rst @@ -0,0 +1,39 @@ +Modifying the Database +====================== + +Sometimes new features require modifying the database, such as adding a new +column. Implementing such changes is fine, but can cause compatibility problems +for users who are using an older version of the database schema. To resolve +this issue, we use `alembic `_ to handle database migrations. + +To use alembic to migrate the database schema, first run the following +command:: + + python -m nbgrader.dbutil revision -m "a description of the change" + +This will create a file in the directory ``nbgrader/alembic/versions``, for +example ``nbgrader/alembic/versions/7685cbef311a_foo.py``. You will now need to +edit the ``upgrade()`` and ``downgrade()`` functions in this file such that +they appropriately make the database changes. For example, to add a column +called ``extra_credit`` to the ``grade`` table: + +.. code:: python + + def upgrade(): + op.add_column('grade', sa.Column('extra_credit', sa.Float)) + + def downgrade(): + op.drop_column('grade', 'extra_credit') + +Please see the `alembic documentation +`_ for further details on +how these files work. Additionally, note that you both need to update the +database schema in ``nbgrader/api.py`` (this describes how to create **new** +databases) as well as using alembic to describe what changes need to be made to +**old** databases. + +You can test whether the database migration works appropriately by running:: + + nbgrader db upgrade + +on an old version of the database. diff --git a/nbgrader/docs/source/index.rst b/nbgrader/docs/source/index.rst index ee961880c..e54e44445 100644 --- a/nbgrader/docs/source/index.rst +++ b/nbgrader/docs/source/index.rst @@ -41,6 +41,7 @@ nbgrader contributor_guide/testing contributor_guide/documentation contributor_guide/js_dependencies + contributor_guide/database contributor_guide/releasing .. toctree:: From b18c8579a915ba775f61d33e0eace3cf338d65f5 Mon Sep 17 00:00:00 2001 From: "Jessica B. Hamrick" Date: Thu, 1 Jun 2017 19:12:32 -0700 Subject: [PATCH 10/10] Create new databases with most recent alembic version --- .coveragerc | 3 ++- nbgrader/api.py | 17 +++++++++++++++++ nbgrader/apps/dbapp.py | 9 +++------ nbgrader/tests/apps/test_nbgrader_db.py | 17 ++++++++++++++++- 4 files changed, 38 insertions(+), 8 deletions(-) diff --git a/.coveragerc b/.coveragerc index 12a3f7d5d..f3f76113b 100644 --- a/.coveragerc +++ b/.coveragerc @@ -7,4 +7,5 @@ source = nbgrader omit = /home/travis/virtualenv/python3.4.2/lib/python3.4/site-packages/* nbgrader/tests/* - nbgrader/apps/notebookapp.py \ No newline at end of file + nbgrader/apps/notebookapp.py + nbgrader/alembic diff --git a/nbgrader/api.py b/nbgrader/api.py index 33c2d3ed7..0b3ec079d 100644 --- a/nbgrader/api.py +++ b/nbgrader/api.py @@ -3,6 +3,7 @@ from . import utils import contextlib +import subprocess as sp from sqlalchemy import (create_engine, ForeignKey, Column, String, Text, DateTime, Interval, Float, Enum, UniqueConstraint, Boolean) @@ -15,6 +16,7 @@ from sqlalchemy import select, func, exists, case, literal_column from uuid import uuid4 +from .dbutil import _temp_alembic_ini Base = declarative_base() @@ -23,6 +25,13 @@ def new_uuid(): return uuid4().hex +def get_alembic_version(): + with _temp_alembic_ini('sqlite:////tmp/gradebook.db') as alembic_ini: + output = sp.check_output(['alembic', '-c', alembic_ini, 'heads']) + head = output.decode().split("\n")[0].split(" ")[0] + return head + + class InvalidEntry(ValueError): pass @@ -1029,8 +1038,16 @@ def __init__(self, db_url): self.db = scoped_session(sessionmaker(autoflush=True, bind=self.engine)) # this creates all the tables in the database if they don't already exist + db_exists = len(self.engine.table_names()) > 0 Base.metadata.create_all(bind=self.engine) + # set the alembic version if it doesn't exist + if not db_exists: + alembic_version = get_alembic_version() + self.db.execute("CREATE TABLE alembic_version (version_num VARCHAR(32) NOT NULL);") + self.db.execute("INSERT INTO alembic_version (version_num) VALUES ('{}');".format(alembic_version)) + self.db.commit() + def __enter__(self): return self diff --git a/nbgrader/apps/dbapp.py b/nbgrader/apps/dbapp.py index 3170d5db0..807094eb6 100644 --- a/nbgrader/apps/dbapp.py +++ b/nbgrader/apps/dbapp.py @@ -394,14 +394,11 @@ class DbUpgradeApp(NbGrader): def _backup_db_file(self, db_file): """Backup a database file""" if not os.path.exists(db_file): - return + with Gradebook("sqlite:///{}".format(db_file)): + pass - timestamp = datetime.now().strftime('.%Y-%m-%d-%H%M%S') + timestamp = datetime.now().strftime('.%Y-%m-%d-%H%M%S.%f') backup_db_file = db_file + timestamp - for i in range(1, 10): - if not os.path.exists(backup_db_file): - break - backup_db_file = '{}.{}.{}'.format(db_file, timestamp, i) if os.path.exists(backup_db_file): self.fail("backup db file already exists: %s" % backup_db_file) diff --git a/nbgrader/tests/apps/test_nbgrader_db.py b/nbgrader/tests/apps/test_nbgrader_db.py index 4918feee5..a2622f265 100644 --- a/nbgrader/tests/apps/test_nbgrader_db.py +++ b/nbgrader/tests/apps/test_nbgrader_db.py @@ -285,7 +285,22 @@ def test_assignment_import(self, db, temp_cwd): assignment = gb.find_assignment("bar") assert assignment.duedate is None - def test_upgrade_db(self, course_dir): + def test_upgrade_nodb(self, temp_cwd): + # test upgrading without a database + run_nbgrader(["db", "upgrade"]) + + def test_upgrade_current_db(self, course_dir): + # add assignment files + self._copy_file(join("files", "test.ipynb"), join(course_dir, "source", "ps1", "p1.ipynb")) + self._copy_file(join("files", "test.ipynb"), join(course_dir, "source", "ps1", "p2.ipynb")) + + # check that nbgrader assign fails + run_nbgrader(["assign", "ps1", "--create"]) + + # test upgrading with a current database + run_nbgrader(["db", "upgrade"]) + + def test_upgrade_old_db(self, course_dir): # add assignment files self._copy_file(join("files", "test.ipynb"), join(course_dir, "source", "ps1", "p1.ipynb")) self._copy_file(join("files", "test.ipynb"), join(course_dir, "source", "ps1", "p2.ipynb"))