help wanted: porting sqlalchemy-migrate for SQLAlchemy 0.7 and sqlalchemy-migrate maintenance
Jan Dittberner
I work on making sqlalchemy-migrate [1] work with SQLAlchemy 0.7. I
fixed all broken unit tests except for one related to adding a new
column with a foreign key to an existing table. We have a continues
integration system (Jenkins CI) at [2] that provides the output of the
failing test. The problem is with some changed behaviour of the
SchemaVisitor API (or the Column objects). Until SQLAlchemy 0.6 it was
possible to get the constraints object for the ForeignKey arguments of
a column. The test at [3] creates a new Column instance and adds it to
the table. Afterwards our ANSIColumnGenerator [4] is triggered to
generate the necessary SQL statements. Until SQLAlchemy 0.6 the code
for generating the foreign key constraints could be generated properly
but now the constraint is None instead of a ForeignKeyConstraint. I
tried to just ignore fk.constraint if it is None, which expectedly did
not generate a statement. I also tried to construct a
ForeignKeyConstraint instance and pass that to the AddConstraint
constructor. This approach added a second ForeignKey instance to the
Column which is not desired too.
Can you please give me hints in the right direction or provide help to
fix this issue?
We would also like to invite interested developers to join the
sqlalchemy-migrate project because it has no maintainers with enough
time to keep it in a good shape. I think it would be great if the test
coverage and code quality would be improved but neither me nor the
other current maintainers have enough time to do these necessary
prerequisites. We have a quite long list of outstanding issues [5]
that need some triaging and fixes and should give a good start for
interested developers.
[1] http://code.google.com/p/sqlalchemy-migrate/
[2] http://jenkins.gnuviech-server.de/job/sqlalchemy-migrate-all/
[3] http://code.google.com/p/sqlalchemy-migrate/source/browse/migrate/tests/changeset/test_changeset.py#160
[4] http://code.google.com/p/sqlalchemy-migrate/source/browse/migrate/changeset/ansisql.py#87
[5] http://code.google.com/p/sqlalchemy-migrate/issues/list
Regards
Jan Dittberner
Michael Bayer
On May 26, 2011, at 11:41 AM, Jan Dittberner wrote:
> Hello,
>
> I work on making sqlalchemy-migrate [1] work with SQLAlchemy 0.7. I
> fixed all broken unit tests except for one related to adding a new
> column with a foreign key to an existing table. We have a continues
> integration system (Jenkins CI) at [2] that provides the output of the
> failing test. The problem is with some changed behaviour of the
> SchemaVisitor API (or the Column objects). Until SQLAlchemy 0.6 it was
> possible to get the constraints object for the ForeignKey arguments of
> a column. The test at [3] creates a new Column instance and adds it to
> the table.
the linked samples don't make it clear what specific behavior in SQLAlchemy has changed. From your description, it appears as though you are saying ForeignKeyConstraint is not generated for an append_column() operation. Below is a test which illustrates this usage, it is the same in 0.6 and 0.7. If you can alter this test case to illustrate the specific functionality that has changed on the SQLA side from 0.6 to 0.7, it may very well be a bug in 0.7 or some usage in Migrate that was never supported, but at the very least it would isolate the issue.
from sqlalchemy import Table, Column, ForeignKey, MetaData, Integer, ForeignKeyConstraint
m1 = MetaData()
t1 = Table('t1', m1,
Column('x', Integer, primary_key=True)
)
t2 = Table('t2', m1,
Column('x', Integer, primary_key=True),
Column('added_inline', Integer, ForeignKey('t1.x'))
)
assert set([
fkc.columns[0].name
for fkc in t2.constraints
if isinstance(fkc, ForeignKeyConstraint)]) == set(['added_inline'])
t2.append_column(
Column('added_externally', Integer, ForeignKey('t1.x'))
)
assert set([
fkc.columns[0].name
for fkc in t2.constraints
if isinstance(fkc, ForeignKeyConstraint)]) == set(['added_externally', 'added_inline'])
Jan Dittberner
the table.append_column() call was the missing piece of information.
In SQLA 0.6 it was not needed, in SQLA 0.7 it seems to be required.
The table.create(col) call triggered the
ANSIColumnGenerator.visit_column(self, column) method and the column
object passed to the method had a ForeignKey instance with ForeignKey
objects' constraint properties set to a ForeignKeyConstraint instance
instead of None (as in SQLA 0.7).
I was able to fix the unit test by adding
self.table.append_column(col) for SQLA 0.7. I hope that the change in
http://code.google.com/p/sqlalchemy-migrate/source/detail?r=2426a14c98f556fc63be2d677378255bfd8e4c2f
is correct and does not only fix the test.
Regards
Jan Dittberner
Michael Bayer
On May 26, 2011, at 4:04 PM, Jan Dittberner wrote:
> the table.append_column() call was the missing piece of information.
> In SQLA 0.6 it was not needed, in SQLA 0.7 it seems to be required.
the Column is guaranteed not at all associated with a Table if you didn't call table.append_column(col). Its a free standing object that's not in a meaningful state when used in a SQL schema context.
> The table.create(col) call triggered the
> ANSIColumnGenerator.visit_column(self, column) method and the column
> object passed to the method had a ForeignKey instance with ForeignKey
> objects' constraint properties set to a ForeignKeyConstraint instance
> instead of None (as in SQLA 0.7).
errrr theres nothing I can see in the source that looks anything like that . ForeignKey is virtually identical in 0.6 and 0.7. The .constraint attribute is created when the ForeignKey is associated with a parent Table which occurs via the Column. I can't yet figure out a test that would behave differently on 0.6 vs. 0.7.
>
> I was able to fix the unit test by adding
> self.table.append_column(col) for SQLA 0.7. I hope that the change in
> http://code.google.com/p/sqlalchemy-migrate/source/detail?r=2426a14c98f556fc63be2d677378255bfd8e4c2f
> is correct and does not only fix the test.
well if this is only the test that relies upon the table + col issue then you are probably OK.