Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQLAlchemyBackend postgresql initialize error #64

Open
motord opened this issue Mar 22, 2014 · 9 comments
Open

SQLAlchemyBackend postgresql initialize error #64

motord opened this issue Mar 22, 2014 · 9 comments
Assignees
Labels

Comments

@motord
Copy link

motord commented Mar 22, 2014

For db_full_url like 'postgresql://user:password@localhost/database', initialization will fail.

        if initialize:
            # Create new database if needed.
            db_url, db_name = db_full_url.rsplit('/', 1)
            self._engine = create_engine(db_url)
            try:
                self._engine.execute("CREATE DATABASE %s" % db_name)
            except Exception, e:
                log.info("Failed DB creation: %s" % e)

            # SQLite in-memory database URL: "sqlite://:memory:"
            if db_name != ':memory:':
                self._engine.execute("USE %s" % db_name)
@thirashima
Copy link

It's the "USE" statement (line 148, in sqlalchemy_backend.py). Postgres always connects to a database using the connect string, so the USE is not needed. A possible fix would be to change the "if" statement that checks for ":memory:" to check to see if it is postgres as well.

@jlutz777
Copy link
Contributor

jlutz777 commented Dec 3, 2014

To implement this properly, it should be broken out into its own class instead of the inline postgres-specific code, but for those that need it, this is my Postgres workaround. I wrote some unit tests, and it seems to work for Postgresql. As a warning, I did NOT test this thoroughly with other database types to make sure I didn't break anything.

    if initialize:
        # Create new database if needed.
        db_url, db_name = db_full_url.rsplit('/', 1)
        isPostgres = False

        # Postgresql needs to be treated differently because it always
        # connects to a database ... template1 is used as a dummy db
        # in order to create the db we want
        if db_url.startswith('postgresql'):
            isPostgres = True
            db_url += '/template1'

        self._engine = create_engine(db_url)

        # Postgresql won't let you create a database in a transaction
        if isPostgres:
            self._engine.raw_connection().set_isolation_level(0)

        try:
            self._engine.execute("CREATE DATABASE %s" % db_name)
        except Exception, e:
            log.info("Failed DB creation: %s" % e)

        # Postgresql doesn't use the USE statement, but make
        # sure you set it back to using translations
        if isPostgres:
            self._engine.raw_connection().set_isolation_level(1)
        # SQLite in-memory database URL: "sqlite://:memory:"
        elif db_name != ':memory:':
            self._engine.execute("USE %s" % db_name)

    else:
        self._engine = create_engine(db_full_url)

@astubbs
Copy link

astubbs commented Aug 4, 2015

Whipped up some commits based on the above, let me know what you think: astubbs@ee6da49

@astubbs
Copy link

astubbs commented Aug 4, 2015

The tests don't seem to be self contained in tox. What's the process for getting this tested properly?

@astubbs
Copy link

astubbs commented Aug 4, 2015

Ignore that sha, the branch is here: https://github.com/astubbs/bottle-cork/tree/postgres-fix

@jlutz777
Copy link
Contributor

jlutz777 commented Aug 4, 2015

I didn't test it, but your branch doesn't look quite right. You are calling create_engine twice for postgresql and not setting the isolation level back. It looks like a copy-paste mistake.

@astubbs
Copy link

astubbs commented Aug 4, 2015

No, I added it on purpose. How can you create the database, and connect to it - I understood you have to connect directly to the db, so you'd need to reconnect, no?
But yes you were right about turning the transactions back on, I've put that back in.
Do you know if the tests should all pass in origin/master by just simply running tox?

commits: https://github.com/astubbs/bottle-cork/commits/postgres-fix

@jlutz777
Copy link
Contributor

jlutz777 commented Aug 4, 2015

I see what you did now, sorry about that. As far as the tests, I'm not sure. As you can guess since I posted this a while ago, I didn't get around to actually implementing it and figuring out the tests. Maybe Federico can help with some direction on running the tests.

@botzill
Copy link

botzill commented Apr 14, 2016

Hi.

I wanted to know what is the status of this issue? Was it fixed/merged? I see there is some fix on it.

Thx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants