)]}'
{"designate/objects/domain.py":[{"author":{"_account_id":741,"name":"Kiall Mac Innes","email":"kiall@macinnes.ie","username":"kiall"},"change_message_id":"cfe5336ed87600398a665774fe7285dde1d1158e","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        \u0027status\u0027: {},"},{"line_number":33,"context_line":"        \u0027action\u0027: {},"},{"line_number":34,"context_line":"        \u0027pool_id\u0027: {},"},{"line_number":35,"context_line":"        \u0027reverse_name\u0027: {},"},{"line_number":36,"context_line":"        \u0027recordsets\u0027: {"},{"line_number":37,"context_line":"            \u0027relation\u0027: True"},{"line_number":38,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_9ce28c2d","line":35,"updated":"2014-11-13 23:03:46.000000000","message":"Ideally, this wouldn\u0027t be exposed outside of the SQLA driver, which probably means updating the SQAL object from model method to ignore columns which don\u0027t have a matching object field.","commit_id":"8469e30ca26e8f0fce11ed8d6af299b38fe68b73"},{"author":{"_account_id":8174,"name":"Tim Simmons","email":"tim.r.simmons@gmail.com","username":"TimSimmons"},"change_message_id":"b0f4e9ccf4df18993178a8ba64d66b7a49bf9f22","unresolved":false,"context_lines":[{"line_number":32,"context_line":"        \u0027status\u0027: {},"},{"line_number":33,"context_line":"        \u0027action\u0027: {},"},{"line_number":34,"context_line":"        \u0027pool_id\u0027: {},"},{"line_number":35,"context_line":"        \u0027reverse_name\u0027: {},"},{"line_number":36,"context_line":"        \u0027recordsets\u0027: {"},{"line_number":37,"context_line":"            \u0027relation\u0027: True"},{"line_number":38,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_3a5b582f","line":35,"in_reply_to":"5a890539_9ce28c2d","updated":"2014-11-13 23:45:28.000000000","message":"Cool, I wasn\u0027t happy with this way myself.","commit_id":"8469e30ca26e8f0fce11ed8d6af299b38fe68b73"}],"designate/storage/impl_sqlalchemy/__init__.py":[{"author":{"_account_id":741,"name":"Kiall Mac Innes","email":"kiall@macinnes.ie","username":"kiall"},"change_message_id":"cfe5336ed87600398a665774fe7285dde1d1158e","unresolved":false,"context_lines":[{"line_number":257,"context_line":"    def _find_domains(self, context, criterion, one\u003dFalse, marker\u003dNone,"},{"line_number":258,"context_line":"                      limit\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":259,"context_line":"        if criterion:"},{"line_number":260,"context_line":"            if criterion.get(\u0027name\u0027, \"\").startswith(\u0027*\u0027):"},{"line_number":261,"context_line":"                criterion[\u0027reverse_name\u0027] \u003d criterion.pop(\u0027name\u0027)[::-1]"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        return self._find("}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_fc5630cd","line":260,"updated":"2014-11-13 23:03:46.000000000","message":"Nit - since theres another review needed anyway - Ideally, we would have a single if statement here using an and.. I\u0027m sure its personal preferance, but I tend to find nested if\u0027s harder to grasp without \"actually reading\".. If that makes any sense ;)","commit_id":"8469e30ca26e8f0fce11ed8d6af299b38fe68b73"},{"author":{"_account_id":8174,"name":"Tim Simmons","email":"tim.r.simmons@gmail.com","username":"TimSimmons"},"change_message_id":"b0f4e9ccf4df18993178a8ba64d66b7a49bf9f22","unresolved":false,"context_lines":[{"line_number":257,"context_line":"    def _find_domains(self, context, criterion, one\u003dFalse, marker\u003dNone,"},{"line_number":258,"context_line":"                      limit\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":259,"context_line":"        if criterion:"},{"line_number":260,"context_line":"            if criterion.get(\u0027name\u0027, \"\").startswith(\u0027*\u0027):"},{"line_number":261,"context_line":"                criterion[\u0027reverse_name\u0027] \u003d criterion.pop(\u0027name\u0027)[::-1]"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        return self._find("}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_5f7656d8","line":260,"in_reply_to":"5a890539_fc5630cd","updated":"2014-11-13 23:45:28.000000000","message":"Agreed.","commit_id":"8469e30ca26e8f0fce11ed8d6af299b38fe68b73"},{"author":{"_account_id":741,"name":"Kiall Mac Innes","email":"kiall@macinnes.ie","username":"kiall"},"change_message_id":"cfe5336ed87600398a665774fe7285dde1d1158e","unresolved":false,"context_lines":[{"line_number":258,"context_line":"                      limit\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":259,"context_line":"        if criterion:"},{"line_number":260,"context_line":"            if criterion.get(\u0027name\u0027, \"\").startswith(\u0027*\u0027):"},{"line_number":261,"context_line":"                criterion[\u0027reverse_name\u0027] \u003d criterion.pop(\u0027name\u0027)[::-1]"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        return self._find("},{"line_number":264,"context_line":"            context, tables.domains, objects.Domain, objects.DomainList,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_dca8948f","line":261,"updated":"2014-11-13 23:03:46.000000000","message":"The ::-1 is a bit cryptic, even for me (I get given out to all the time for cryptic code ;))! Without the nested if, I believe .reverse() will fit for within the 80 char limit.","commit_id":"8469e30ca26e8f0fce11ed8d6af299b38fe68b73"},{"author":{"_account_id":8174,"name":"Tim Simmons","email":"tim.r.simmons@gmail.com","username":"TimSimmons"},"change_message_id":"b0f4e9ccf4df18993178a8ba64d66b7a49bf9f22","unresolved":false,"context_lines":[{"line_number":258,"context_line":"                      limit\u003dNone, sort_key\u003dNone, sort_dir\u003dNone):"},{"line_number":259,"context_line":"        if criterion:"},{"line_number":260,"context_line":"            if criterion.get(\u0027name\u0027, \"\").startswith(\u0027*\u0027):"},{"line_number":261,"context_line":"                criterion[\u0027reverse_name\u0027] \u003d criterion.pop(\u0027name\u0027)[::-1]"},{"line_number":262,"context_line":""},{"line_number":263,"context_line":"        return self._find("},{"line_number":264,"context_line":"            context, tables.domains, objects.Domain, objects.DomainList,"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_fa547000","line":261,"in_reply_to":"5a890539_dca8948f","updated":"2014-11-13 23:45:28.000000000","message":"There actually isn\u0027t a .reverse() for strings, you do \"\".join(reversed(s)) or the slice (unless I\u0027ve gone mad). A comment perhaps?","commit_id":"8469e30ca26e8f0fce11ed8d6af299b38fe68b73"},{"author":{"_account_id":8174,"name":"Tim Simmons","email":"tim.r.simmons@gmail.com","username":"TimSimmons"},"change_message_id":"1d78588f8e317efe1713885bdf5b8ecd3ee46915","unresolved":false,"context_lines":[{"line_number":264,"context_line":"            exceptions.DomainNotFound, criterion, one, marker, limit,"},{"line_number":265,"context_line":"            sort_key, sort_dir)"},{"line_number":266,"context_line":""},{"line_number":267,"context_line":"    def create_domain(self, context, domain):"},{"line_number":268,"context_line":"        #Add the required field to the object"},{"line_number":269,"context_line":"        domain.FIELDS[\u0027reverse_name\u0027] \u003d {}"},{"line_number":270,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_59dad24c","line":267,"updated":"2014-11-14 20:23:40.000000000","message":"How do we feel about this logic, still seems a bit hacky to me?","commit_id":"28c336ecc908ac7f09fb88dcc5960353baada16c"},{"author":{"_account_id":8130,"name":"Betsy Luzader","email":"betsy.luzader@rackspace.com","username":"bluzader"},"change_message_id":"26ad5265c0536327e7afde17e7bfd635eaaae7c3","unresolved":false,"context_lines":[{"line_number":273,"context_line":""},{"line_number":274,"context_line":"        #Inform the object of the changes"},{"line_number":275,"context_line":"        domain._obj_changes.add(u\"reverse_name\")"},{"line_number":276,"context_line":""},{"line_number":277,"context_line":"        # Don\u0027t handle recordsets for now"},{"line_number":278,"context_line":"        return self._create("},{"line_number":279,"context_line":"            tables.domains, domain, exceptions.DuplicateDomain, [\u0027recordsets\u0027])"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_8fb788f4","line":276,"updated":"2014-11-14 21:57:15.000000000","message":"Do we need to add the \u0027u\u0027? Did you run into a problem without it?","commit_id":"28c336ecc908ac7f09fb88dcc5960353baada16c"},{"author":{"_account_id":741,"name":"Kiall Mac Innes","email":"kiall@macinnes.ie","username":"kiall"},"change_message_id":"427318ab274f4e6140d2078f8675836cd409680a","unresolved":false,"context_lines":[{"line_number":751,"context_line":"        }"},{"line_number":752,"context_line":""},{"line_number":753,"context_line":"    # Reverse Name utils"},{"line_number":754,"context_line":"    def rname_obj_patch(self, obj):"},{"line_number":755,"context_line":"        # Add the required field to the object"},{"line_number":756,"context_line":"        obj.FIELDS[\u0027reverse_name\u0027] \u003d {}"},{"line_number":757,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_3b986c77","line":754,"updated":"2014-11-17 20:29:51.000000000","message":"Copy+Paste from IRC:\n\n    20:27 \u003cKiall\u003e timsim: Cool, 1 last thing on that review, rather than doing this kinda thing: obj.FIELDS[\u0027reverse_name\u0027] \u003d {}\n    20:27 \u003cKiall\u003e \n    20:27 \u003cKiall\u003e can we instead add a more generic \"extra_values\" which would look like \"skip_values\" does? e.g. https://github.com/openstack/designate/blob/master/designate/sqlalchemy/base.py#L238\n    20:28 \u003cKiall\u003e \"obj.FIELDS[\u0027reverse_name\u0027] \u003d {}\" will \"leak\" back out of the storage layer when the object get\u0027s returned etc","commit_id":"1243b061ea2841920199a46abbc950cdd34a5086"},{"author":{"_account_id":741,"name":"Kiall Mac Innes","email":"kiall@macinnes.ie","username":"kiall"},"change_message_id":"427318ab274f4e6140d2078f8675836cd409680a","unresolved":false,"context_lines":[{"line_number":761,"context_line":"        # Inform the object of the changes"},{"line_number":762,"context_line":"        obj._obj_changes.add(u\"reverse_name\")"},{"line_number":763,"context_line":""},{"line_number":764,"context_line":"    def rname_check(self, criterion):"},{"line_number":765,"context_line":"        if criterion is not None and criterion.get(\u0027name\u0027, \"\").startswith(\u0027*\u0027):"},{"line_number":766,"context_line":"                criterion[\u0027reverse_name\u0027] \u003d criterion.pop(\u0027name\u0027)[::-1]"},{"line_number":767,"context_line":"        return criterion"}],"source_content_type":"text/x-python","patch_set":5,"id":"5a890539_fbaa6483","line":764,"updated":"2014-11-17 20:29:51.000000000","message":"This should be a private method (e.g. def _rname_check())","commit_id":"1243b061ea2841920199a46abbc950cdd34a5086"}],"designate/storage/impl_sqlalchemy/migrate_repo/versions/046_add_reverse_name.py":[{"author":{"_account_id":741,"name":"Kiall Mac Innes","email":"kiall@macinnes.ie","username":"kiall"},"change_message_id":"cfe5336ed87600398a665774fe7285dde1d1158e","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    reverse_name_col.create(domains_table)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    migrate_engine.execute(text("},{"line_number":31,"context_line":"        \"UPDATE domains SET reverse_name\u003dREVERSE(name);\"))"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    rev_ind \u003d Index(\u0027reverse_name_deleted\u0027, domains_table.c.reverse_name,"},{"line_number":34,"context_line":"         domains_table.c.deleted)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_9cbfeca9","line":31,"updated":"2014-11-13 23:03:46.000000000","message":"We should be able to use SQLA\u0027s methods here instead of hand writing the query.. The reasoning being, I have no clue if, for example, DB2 uses the same SQL syntax for this.. But SQLA probably does...","commit_id":"8469e30ca26e8f0fce11ed8d6af299b38fe68b73"},{"author":{"_account_id":8174,"name":"Tim Simmons","email":"tim.r.simmons@gmail.com","username":"TimSimmons"},"change_message_id":"b0f4e9ccf4df18993178a8ba64d66b7a49bf9f22","unresolved":false,"context_lines":[{"line_number":28,"context_line":"    reverse_name_col.create(domains_table)"},{"line_number":29,"context_line":""},{"line_number":30,"context_line":"    migrate_engine.execute(text("},{"line_number":31,"context_line":"        \"UPDATE domains SET reverse_name\u003dREVERSE(name);\"))"},{"line_number":32,"context_line":""},{"line_number":33,"context_line":"    rev_ind \u003d Index(\u0027reverse_name_deleted\u0027, domains_table.c.reverse_name,"},{"line_number":34,"context_line":"         domains_table.c.deleted)"}],"source_content_type":"text/x-python","patch_set":1,"id":"5a890539_faa2b0fb","line":31,"in_reply_to":"5a890539_9cbfeca9","updated":"2014-11-13 23:45:28.000000000","message":"Agree 100%. I didn\u0027t get it quickly so I just decided to put this up before I went home :)","commit_id":"8469e30ca26e8f0fce11ed8d6af299b38fe68b73"},{"author":{"_account_id":8174,"name":"Tim Simmons","email":"tim.r.simmons@gmail.com","username":"TimSimmons"},"change_message_id":"1d78588f8e317efe1713885bdf5b8ecd3ee46915","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    domains_table \u003d Table(\u0027domains\u0027, meta, autoload\u003dTrue)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    reverse_name_col \u003d Column(\u0027reverse_name\u0027, String(255), nullable\u003dFalse,"},{"line_number":28,"context_line":"                              server_default\u003d\u0027\u0027)"},{"line_number":29,"context_line":"    reverse_name_col.create(domains_table)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    zones \u003d select(columns\u003d[domains_table.c.id, domains_table.c.name])\\"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_7924ee5f","line":28,"updated":"2014-11-14 20:23:40.000000000","message":"Didn\u0027t like this, but you can\u0027t add a non-nullable column without a default (that isn\u0027t null).","commit_id":"28c336ecc908ac7f09fb88dcc5960353baada16c"},{"author":{"_account_id":8130,"name":"Betsy Luzader","email":"betsy.luzader@rackspace.com","username":"bluzader"},"change_message_id":"26ad5265c0536327e7afde17e7bfd635eaaae7c3","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    domains_table \u003d Table(\u0027domains\u0027, meta, autoload\u003dTrue)"},{"line_number":26,"context_line":""},{"line_number":27,"context_line":"    reverse_name_col \u003d Column(\u0027reverse_name\u0027, String(255), nullable\u003dFalse,"},{"line_number":28,"context_line":"                              server_default\u003d\u0027\u0027)"},{"line_number":29,"context_line":"    reverse_name_col.create(domains_table)"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    zones \u003d select(columns\u003d[domains_table.c.id, domains_table.c.name])\\"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_6f0dfce7","line":28,"in_reply_to":"5a890539_7924ee5f","updated":"2014-11-14 21:57:15.000000000","message":"You can create it as nullable\u003dTrue. Then after you put in all the values,  you can alter the table to be nullable\u003dFalse. That avoids putting in an empty string","commit_id":"28c336ecc908ac7f09fb88dcc5960353baada16c"}],"designate/storage/impl_sqlalchemy/tables.py":[{"author":{"_account_id":8130,"name":"Betsy Luzader","email":"betsy.luzader@rackspace.com","username":"bluzader"},"change_message_id":"26ad5265c0536327e7afde17e7bfd635eaaae7c3","unresolved":false,"context_lines":[{"line_number":104,"context_line":"           default\u003d\u0027CREATE\u0027, server_default\u003d\u0027CREATE\u0027, nullable\u003dFalse),"},{"line_number":105,"context_line":"    Column(\u0027pool_id\u0027, UUID, default\u003dNone, nullable\u003dTrue),"},{"line_number":106,"context_line":"    Column(\u0027reverse_name\u0027, String(255), nullable\u003dFalse, default\u003d\u0027\u0027),"},{"line_number":107,"context_line":""},{"line_number":108,"context_line":"    UniqueConstraint(\u0027name\u0027, \u0027deleted\u0027, name\u003d\u0027unique_domain_name\u0027),"},{"line_number":109,"context_line":"    ForeignKeyConstraint([\u0027parent_domain_id\u0027],"},{"line_number":110,"context_line":"                         [\u0027domains.id\u0027],"}],"source_content_type":"text/x-python","patch_set":3,"id":"5a890539_0ffe38ae","line":107,"updated":"2014-11-14 21:57:15.000000000","message":"Then you don\u0027t need to include the default value here at all.","commit_id":"28c336ecc908ac7f09fb88dcc5960353baada16c"}]}