arskom
Spyne PGObjectXml columns don't work with SQLAlchemy reflection
My particular case is with the sqlalchemy migration tool alembic but this looks to be a general issue with PGObjectXml, its sister classes and reflection. The initial migration works fine (with a minor issue with the auto-generation) but after there are xml columns in the database sqlalchemy reflection no longer works as it isn't able to instantiate a spyne.util.sqlalchemy.PGObjectXml object.
The PGObjectXml.init expects two args (self, cls) and of course sqlalchemy reflection doesn't pass cls and so throws a TypeError. I have overcome this error for now with making the cls parameter of PGObjectXml.init have a default of None, a dumb solution but one that allows my development to proceed.
The alembic code never uses the reflected columns or calls the members of PGObjectXml that need a non-None value for cls. This probably isn't the case for all uses of PGObjectXml + reflection but it covers mine.
I have an test case for this bug here: https://github.com/hartror/spyne-alembic-bug
My thoughts on fixing this would be to make cls with a None default and check it isn't None when PGObjectXml members that need cls set are called. If they are called with cls == None then throw an exception.
I am more than happy to take a crack at fixing the issue as we're using spyne in a project and apart from this we've had great success using it.
Hello,
First, thank you very much for the excellent bug report.
The good news is: I've already fixed this :) The patch was a bit messy (and adds new SQLAlchemy types) so I did not commit it to 2.10 branch.
It doesn't try to guess what object the PGObject* was trying to store and reflects these types of columns as PGJson or PGXml though. That information is just not in the database. Do you think that's a good solution?
Anything else I can do for you?
Best regards,
The workaround is this line: https://github.com/arskom/spyne/blob/97ba5c7cfde2e9428705f8eacbf08fc07e107669/spyne/util/sqlalchemy.py#L238
I did not try, but I think you can override this in your own code, without needing to patch Spyne.
Thanks for you prompt response!
Unfortunately testing against my test case with the latest version of spyne causes a different but related issue. When running step 4 in the test case README.txt there is a difference detect between what Array(Unicode).store_as('xml') generates as a column and what sqlalchemy generates as the reflected column.
$ alembic revision --autogenerate -m "added new column" INFO [alembic.migration] Context impl PostgresqlImpl. INFO [alembic.migration] Will assume transactional DDL. INFO [alembic.autogenerate] Detected added column 'examples.new_column' INFO [alembic.autogenerate] Detected type change from PGXml() to PGObjectXml(<class 'spyne.model.complex.Array'>) on 'examples.a_list' Generating /home/roryhart/personal/spyne-alembic- bug/alembic/versions/199832d88e7b_added_new_column.py...done
I had to update the test case to generate this error as alembic doesn't detect type changes by default. The project I am working on uses this functionality of alembic's.
Is there any reason we couldn't merge the two sets of XML & JSON classes together? Or a better solution if possible, is for them to share a common ancestor which makes them visible to SA as a single type but with differing implementations. Which is assuming something like that is possible with the user-defined types.
I've updated the test case and tested it with the latest code on master.
hmmm, interesting.
I think PGObjectJson could inherit from PGJson and likewise for PGXml and PGObjectXml. I wouldn't prefer to merge *Xml and *Json classes though. would that work?
Btw, I'm yet to try the new test case.
That is what I was thinking. I will give it a try in the morning (10pm here now and my fiance is getting annoyed ;) ). Thanks.