Skip to content

sqlalchemy scaffold instructions are confusing #481

Closed
mcdonc opened this Issue Mar 14, 2012 · 6 comments

6 participants

@mcdonc
Pylons Project member
mcdonc commented Mar 14, 2012

Currently when someone uses the pcreate script to create a sqlachemy-scaffolded application, this happens:

  Copying setup.py_tmpl to /home/chrism/projects/pyramid/env26/foobar/setup.py
Welcome to Pyramid.  Sorry for the convenience.

Please run the "populate_foobar" script to set up the SQL database after
installing (but before starting) the application.

 For example:

cd /home/chrism/projects/pyramid/env26/foobar
/home/chrism/projects/pyramid/env26/bin/python setup.py develop
/home/chrism/projects/pyramid/env26/bin/populate_foobar development.ini

This is really confusing for new users due to chronology. Instead of doing what we do now, maybe we stop emitting the console warning after "pcreate" and handle the error at runtime:

from pyramid.view import view_config

from .models import (
    DBSession,
    MyModel,
    )

@view_config(route_name='home', renderer='templates/mytemplate.pt')
def my_view(request):
    try:
        one = DBSession.query(MyModel).filter(MyModel.name=='one').first()
    except sqlalchemy.exc.OperationalError:
        return Response('You haven't run the populate script yet')
    return {'one':one, 'project':'{{project}}'}

Any better ideas?

@shentonfreude

I agree with this. When walking through the tutorial yesterday, I -- of course -- tried to do as directed by the message and it failed. I just pushed on with the tutorial and it later told me how to populate the DB correctly.

You solution seems fine: fail at turn time.

@mmerickel
Pylons Project member

So a possible solution, and (imo) a best practice, I would update the scaffold to use http://pypi.python.org/pypi/alembic. This would initialize the migration table in the populate script, and then have the app check upon startup to ensure that the database is at the latest version. If the database is out of date (or doesn't exist) it would throw an error with a message explaining how to fix it. I realize it's another dependency and complexity, but it makes it possible to setup the database for migrations from the start of the scaffold. Maybe this is too much, but the basic idea is still to check at startup to ensure the database is initialized which is better than checking if one particular data row is in a table.

@goodwillcoding
Pylons Project member

FWIW, I looked at how Django handles this and on over all its fairly complicated (syncdb, flush, reset, loaddata) and other commands. It is really tied in with the models and does a lot of detection.

I agree with @mmerickel that alembic is the right solution in general. However alembic is pretty complicated.

I would suggest the populate_foobar command to change to ppopulate for consistency for now with more clear instructions.

Additionally, I'd have the scaffold run the command

@mgedmin
mgedmin commented Mar 14, 2012

Emitting the warning on application startup (instead of waiting for the user to try and render a view that needs this particular table) would be a tiny bit nicer, IMHO. Especially if the warning contains explicit commands that can be copied and pasted into the shell.

@mcdonc mcdonc added a commit that referenced this issue Mar 14, 2012
@mcdonc mcdonc Changes to support #481.
- The ``alchemy`` scaffold now shows an informative error message in the
  browser if the person creating the project forgets to run the
  initialization script.

- The ``alchemy`` scaffold initialization script is now called
  ``initialize_<projectname>_db`` instead of ``populate_<projectname>``.
a678380
@mikeorr
Pylons Project member
mikeorr commented Mar 15, 2012

a678380 looks good. I was concerned that the exception suggested in the ticket catches too much, but the implementation correctly says that the problem "may be" missing tables rather than "is" missing tables.

@mcdonc
Pylons Project member
mcdonc commented Sep 17, 2012

Closing; we haven't got any complaints about this since we changed it to emit an error message at runtime if the query fails.

@mcdonc mcdonc closed this Sep 17, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.