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

Doesn't import custom column types like ArrowType from the right place #229

Closed
sqlalchemy-bot opened this Issue Oct 2, 2014 · 15 comments

Comments

Projects
None yet
1 participant
@sqlalchemy-bot

sqlalchemy-bot commented Oct 2, 2014

Migrated issue, originally created by nickretallack (@nickretallack)

I wrote a simple model in Flask-SQLAlchemy like this:

from sqlalchemy_utils.types.arrow import ArrowType
class MyModel(db.Model):
    created_at = db.Column(ArrowType)

Then I generated the migration using Flask-Migrate, which uses Alembic internally, and it generated this:

...
import sqlalchemy as sa
...
    sa.Column('created_at', sa.ArrowType(), nullable=True),
...

Of course, ArrowType is not part of sqlalchemy, so I edited it to import it from the right place. It'd be cool if it got the import correct

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 2, 2014

Changes by nickretallack (@nickretallack):

  • edited description
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 2, 2014

Michael Bayer (@zzzeek) wrote:

see http://alembic.readthedocs.org/en/latest/tutorial.html#autogen-module-prefix , reopen if you still have issues, thanks

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 2, 2014

Michael Bayer (@zzzeek) wrote:

existing feature

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 2, 2014

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 21, 2014

nickretallack (@nickretallack) wrote:

Why can't it just figure out where to import the type from by checking ArrowType.__module__ ?

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 21, 2014

nickretallack (@nickretallack) wrote:

Ok I tried editing my env.py and putting in the user_module_prefix but it didn't change anything. But even if it did, I'm using several custom types from different modules, so how would it work for all of them? This seems pretty shitty :P

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 21, 2014

nickretallack (@nickretallack) wrote:

This could be better. Also, it doesn't work for me.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 21, 2014

Changes by nickretallack (@nickretallack):

  • changed status to reopened
@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 21, 2014

Michael Bayer (@zzzeek) wrote:

you'd need to provide a single module where all your custom types can be imported from. as for "doesn't work" I don't really know what to say, you're saying new autogen file and it spits out the "sa" prefix anyway ? can i have some specifics please ?

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 21, 2014

Michael Bayer (@zzzeek) wrote:

also, I'm an older person, sorry if this is normal for younger folks these days but calling my code "pretty shitty" is not a great way to inspire me to care about changing it for you.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 21, 2014

Michael Bayer (@zzzeek) wrote:

works for me. make a blank alembic environment, use this env.py:

from __future__ import with_statement
from alembic import context
from sqlalchemy import engine_from_config, pool
from logging.config import fileConfig

config = context.config

fileConfig(config.config_file_name)

from sqlalchemy import TypeDecorator, Integer
class MyType(TypeDecorator):
    impl = Integer

from sqlalchemy import MetaData

target_metadata = metadata = MetaData()

def model():
    from sqlalchemy import Column, Integer, String, MetaData, ForeignKey
    from sqlalchemy import Enum, Text, Sequence, BigInteger, DateTime, Boolean
    from sqlalchemy import UniqueConstraint, CheckConstraint
    from sqlalchemy.ext.declarative import declarative_base
    from sqlalchemy.orm import relationship


    Base = declarative_base(metadata=metadata)

    class MyThing(Base):
        __tablename__ = 'foo'
        id = Column(Integer, primary_key=True)
        foo = Column(MyType)

model()


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.

    """
    engine = engine_from_config(
                config.get_section(config.config_ini_section),
                prefix='sqlalchemy.',
                poolclass=pool.NullPool)

    connection = engine.connect()

    context.configure(
                connection=connection,
                target_metadata=target_metadata,
                transaction_per_migration=True,
                user_module_prefix='foob.',
                )

    try:
        with context.begin_transaction():
            context.run_migrations()
    finally:
        connection.close()

run_migrations_online()


command:

python -m alembic.config revision -m "foo" --autogenerate

output migration:

"""foo

Revision ID: 396c19ed5ac2
Revises: None
Create Date: 2014-10-21 12:54:33.575134

"""

# revision identifiers, used by Alembic.
revision = '396c19ed5ac2'
down_revision = None

from alembic import op
import sqlalchemy as sa


def upgrade():
    ### commands auto generated by Alembic - please adjust! ###
    op.create_table('foo',
    sa.Column('id', sa.Integer(), nullable=False),
    sa.Column('foo', foob.MyType(), nullable=True),
    sa.PrimaryKeyConstraint('id')
    )
    ### end Alembic commands ###


def downgrade():
    ### commands auto generated by Alembic - please adjust! ###
    op.drop_table('foo')
    ### end Alembic commands ###

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 21, 2014

Michael Bayer (@zzzeek) wrote:

the reason you want a fixed module for types and not guessing with __module__ is so that your migration files are future-proof. if __module__ is spit out, that assumes that __module__ is the import point of those types (often it is not) and now you can never change that location either. it's not a good practice.

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 21, 2014

Michael Bayer (@zzzeek) wrote:

that said, with the advent of 0.7 and some backwards incompatibility is OK, __module__ is possibly a better default than "sa.".

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 21, 2014

Michael Bayer (@zzzeek) wrote:

  • the original rationale for defaulting the user-defined namespace
    to "sa." was to force users to deal with making sure their custom
    types came from a fixed module somewhere. However, it's not worth
    defending this rationale.

The default value of the
:paramref:.EnvironmentContext.configure.user_module_prefix
parameter is no longer the same as the SQLAlchemy prefix.
When omitted, user-defined types will now use the __module__
attribute of the type class itself when rendering in an
autogenerated module.

fixes #229

44d5eea

@sqlalchemy-bot

This comment has been minimized.

sqlalchemy-bot commented Oct 21, 2014

Changes by Michael Bayer (@zzzeek):

  • changed status to closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment