)]}'
{"oslo_db/sqlalchemy/test_migrations.py":[{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"397d7581676172dbbcc44d16be147da7de015da3","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        if isinstance(meta_type, types.Variant):"},{"line_number":439,"context_line":"            meta_col.type \u003d meta_type.impl"},{"line_number":440,"context_line":"            self.compare_type(ctxt, insp_col, meta_col, insp_type,"},{"line_number":441,"context_line":"                              meta_type.impl)"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        return None  # tells alembic to use the default comparison method"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_c37d63b2","line":440,"updated":"2015-12-16 14:11:04.000000000","message":"hm, wait, shouldn\u0027t we return the result here?","commit_id":"89a5aa7fdcee1b54767d47f4f939c900de0781db"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"84e2ca6e33d366f8f9146de3ba44da58d87b6840","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        if isinstance(meta_type, types.Variant):"},{"line_number":439,"context_line":"            meta_col.type \u003d meta_type.impl"},{"line_number":440,"context_line":"            self.compare_type(ctxt, insp_col, meta_col, insp_type,"},{"line_number":441,"context_line":"                              meta_type.impl)"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        return None  # tells alembic to use the default comparison method"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_569613f1","line":440,"in_reply_to":"5a710552_16b16be1","updated":"2015-12-16 14:25:00.000000000","message":"Ann, if you don\u0027t return anything, Python will implicitly return None for you: http://stackoverflow.com/questions/6000413/return-none-in-python-not-recommended-ho-to-bypass","commit_id":"89a5aa7fdcee1b54767d47f4f939c900de0781db"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"8796c25c67a0fb752a30e3855470cd475b5b5aee","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        if isinstance(meta_type, types.Variant):"},{"line_number":439,"context_line":"            meta_col.type \u003d meta_type.impl"},{"line_number":440,"context_line":"            self.compare_type(ctxt, insp_col, meta_col, insp_type,"},{"line_number":441,"context_line":"                              meta_type.impl)"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        return None  # tells alembic to use the default comparison method"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_d640c3a7","line":440,"in_reply_to":"5a710552_16b16be1","updated":"2015-12-16 14:24:40.000000000","message":"Done","commit_id":"89a5aa7fdcee1b54767d47f4f939c900de0781db"},{"author":{"_account_id":6524,"name":"Henry Gessau","email":"HenryG@gessau.net","username":"gessau"},"change_message_id":"61e1bf9c18cfb86ae428804734aca6a447d0ec2e","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        if isinstance(meta_type, types.Variant):"},{"line_number":439,"context_line":"            meta_col.type \u003d meta_type.impl"},{"line_number":440,"context_line":"            self.compare_type(ctxt, insp_col, meta_col, insp_type,"},{"line_number":441,"context_line":"                              meta_type.impl)"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        return None  # tells alembic to use the default comparison method"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_16b16be1","line":440,"in_reply_to":"5a710552_563853a0","updated":"2015-12-16 14:20:55.000000000","message":"return self.compare_type(...)","commit_id":"89a5aa7fdcee1b54767d47f4f939c900de0781db"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"ed7b86cb036cbb8402f30ae7053835e32a91a8db","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        if isinstance(meta_type, types.Variant):"},{"line_number":439,"context_line":"            meta_col.type \u003d meta_type.impl"},{"line_number":440,"context_line":"            self.compare_type(ctxt, insp_col, meta_col, insp_type,"},{"line_number":441,"context_line":"                              meta_type.impl)"},{"line_number":442,"context_line":""},{"line_number":443,"context_line":"        return None  # tells alembic to use the default comparison method"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a710552_563853a0","line":440,"in_reply_to":"5a710552_c37d63b2","updated":"2015-12-16 14:16:03.000000000","message":"I need to call self.compare_type with proper parameters here, what result I can return here? I can\u0027t return None as meta_type.impl can be boolean and needs special treat as done above.","commit_id":"89a5aa7fdcee1b54767d47f4f939c900de0781db"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"94ffdd84c699989e5e55b21c1eb5eab008e2a8db","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        if isinstance(meta_type, types.Variant):"},{"line_number":439,"context_line":"            orig_type \u003d meta_col.type"},{"line_number":440,"context_line":"            restore \u003d lambda: setattr(meta_col, \u0027type\u0027, orig_type)"},{"line_number":441,"context_line":"            self.addCleanup(restore)"},{"line_number":442,"context_line":"            meta_col.type \u003d meta_type.impl"},{"line_number":443,"context_line":"            return self.compare_type(ctxt, insp_col, meta_col, insp_type,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a7e1126_ac80c347","line":440,"updated":"2015-12-16 20:52:02.000000000","message":"any reason we don\u0027t do it with try: finally? cleanup will occur only when test is cleaned up, leaving models modified for the rest of the test case, which may be not optimal.","commit_id":"8b4afa014f6814839b1acb8ee565424e5f1857f4"},{"author":{"_account_id":9656,"name":"Ihar Hrachyshka","email":"ihrachys@redhat.com","username":"ihrachys","status":"Red Hat Networking Systems Engineer"},"change_message_id":"6f84e5a16720875875f4c7f1710cbe2daa2440f7","unresolved":false,"context_lines":[{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        if isinstance(meta_type, types.Variant):"},{"line_number":439,"context_line":"            orig_type \u003d meta_col.type"},{"line_number":440,"context_line":"            restore \u003d lambda: setattr(meta_col, \u0027type\u0027, orig_type)"},{"line_number":441,"context_line":"            self.addCleanup(restore)"},{"line_number":442,"context_line":"            meta_col.type \u003d meta_type.impl"},{"line_number":443,"context_line":"            return self.compare_type(ctxt, insp_col, meta_col, insp_type,"}],"source_content_type":"text/x-python","patch_set":3,"id":"3a7e1126_2f9045ec","line":440,"in_reply_to":"3a7e1126_ac80c347","updated":"2015-12-16 21:36:54.000000000","message":"Looked at it more closely. Seems like currently we RELY on the fact that models are left modified after calling the method (then we return None, which makes the default implementation compare_type to be called, and it sees the modified type).\n\nI believes the flow is very twisted right now, and we would be better if we don\u0027t rely on models being left modified till the end of the test case.\n\nThe solution would be to never rely on implicit call to default compare_type implementation, instead calling to it explicitly.\n\nFor example, for neutron patch we would have:\n\n    def compare_type(self, ctxt, insp_col, meta_col, insp_type, meta_type):\n        if isinstance(meta_type, types.Variant):\n            orig_type \u003d meta_col.type\n            meta_col.type \u003d meta_type.impl\n            try:\n                return self.compare_type(ctxt, insp_col, meta_col, insp_type,\n                                         meta_type.impl)\n            finally:\n                meta_col.type \u003d orig_type\n        else:\n            ret \u003d super(_TestModelsMigrations, self).compare_type(\n                ctxt, insp_col, meta_col, insp_type, meta_type)\n            if ret is not None:\n                return ret\n            return ctxt.impl.compare_type(insp_col, meta_col)\n\nThis approach isolates the model modification to the very section that makes the comparison decision, and revert the modification right after it\u0027s done. Similar approach could apply to oslo.db patch.\n\nWhat do you think?","commit_id":"8b4afa014f6814839b1acb8ee565424e5f1857f4"},{"author":{"_account_id":11816,"name":"mike bayer","display_name":"Mike Bayer","email":"mike_mp@zzzcomputing.com","username":"zzzeek","status":"Red Hat"},"change_message_id":"74f0919ddfc822db31feff3a7f005fa604accd3a","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        if issubclass(type(meta_type), BOOLEAN_METADATA):"},{"line_number":436,"context_line":"            return not issubclass(type(insp_type), BOOLEAN_SQL)"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        if isinstance(meta_type, types.Variant):"},{"line_number":439,"context_line":"            orig_type \u003d meta_col.type"},{"line_number":440,"context_line":"            meta_col.type \u003d meta_type.impl"},{"line_number":441,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a7e1126_f1b13a48","line":438,"updated":"2015-12-18 14:23:36.000000000","message":"OK well what has to be considered here is that the point of Variant is to provide different types based on different backends.  So if a Variant provides type X for SQLite and type Y for MySQL, here we\u0027re comparing to what the backend DB is giving us, so if we were running against SQLite we\u0027d want to extract that type, and if on MySQL we\u0027d want to extract the other.  That is, meta_type.impl is not specific enough in the general case.  Instead of impl, we can say meta_type.load_dialect_impl(ctxt.dialect).","commit_id":"70203dbf08e9e4753f2705fe618d7c5bd1f32d37"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"7f3f775818ede45423a8c3a7de7a711b477f0c41","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        if issubclass(type(meta_type), BOOLEAN_METADATA):"},{"line_number":436,"context_line":"            return not issubclass(type(insp_type), BOOLEAN_SQL)"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        if isinstance(meta_type, types.Variant):"},{"line_number":439,"context_line":"            orig_type \u003d meta_col.type"},{"line_number":440,"context_line":"            meta_col.type \u003d meta_type.impl"},{"line_number":441,"context_line":"            try:"}],"source_content_type":"text/x-python","patch_set":4,"id":"3a7e1126_59cdd4b5","line":438,"in_reply_to":"3a7e1126_f1b13a48","updated":"2015-12-18 15:12:44.000000000","message":"Done","commit_id":"70203dbf08e9e4753f2705fe618d7c5bd1f32d37"},{"author":{"_account_id":6849,"name":"Roman Podoliaka","email":"roman.podoliaka@gmail.com","username":"rpodolyaka"},"change_message_id":"d384e41085cd7b872c1362e160739dd8c53c2f40","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        if issubclass(type(meta_type), BOOLEAN_METADATA):"},{"line_number":436,"context_line":"            return not issubclass(type(insp_type), BOOLEAN_SQL)"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        if isinstance(meta_type, types.Variant):"},{"line_number":439,"context_line":"            orig_type \u003d meta_col.type"},{"line_number":440,"context_line":"            impl_type \u003d meta_type.load_dialect_impl(ctxt.dialect)"},{"line_number":441,"context_line":"            meta_col.type \u003d impl_type"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a7e1126_c8d9ec23","line":438,"updated":"2015-12-18 16:19:21.000000000","message":"I guess, we could leave a note here on why complex comparison is needed","commit_id":"ad5d4d7972e29c12c2a3c5fdbee68ebd78e53608"},{"author":{"_account_id":7249,"name":"Ann Taraday","email":"akamyshnikova@mirantis.com","username":"AKamyshnikova"},"change_message_id":"8018f94070930fe4b40eb2aa07dcd15aceec1b68","unresolved":false,"context_lines":[{"line_number":435,"context_line":"        if issubclass(type(meta_type), BOOLEAN_METADATA):"},{"line_number":436,"context_line":"            return not issubclass(type(insp_type), BOOLEAN_SQL)"},{"line_number":437,"context_line":""},{"line_number":438,"context_line":"        if isinstance(meta_type, types.Variant):"},{"line_number":439,"context_line":"            orig_type \u003d meta_col.type"},{"line_number":440,"context_line":"            impl_type \u003d meta_type.load_dialect_impl(ctxt.dialect)"},{"line_number":441,"context_line":"            meta_col.type \u003d impl_type"}],"source_content_type":"text/x-python","patch_set":5,"id":"3a7e1126_a971f177","line":438,"in_reply_to":"3a7e1126_c8d9ec23","updated":"2015-12-19 07:29:50.000000000","message":"Done","commit_id":"ad5d4d7972e29c12c2a3c5fdbee68ebd78e53608"}]}