)]}'
{"nova/db/sqlalchemy/api_models.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"bf972b8a55fa26ecf3e457edfb15295300139193","unresolved":false,"context_lines":[{"line_number":71,"context_line":"            nullable\u003dFalse)"},{"line_number":72,"context_line":"    host \u003d Column(String(255), nullable\u003dFalse)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    cell_mapping \u003d relationship(\"CellMapping\", backref\u003d\"hosts\", lazy\u003d\u0027joined\u0027,"},{"line_number":75,"context_line":"                                innerjoin\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_cf0ecd8b","line":74,"updated":"2015-07-01 21:13:08.000000000","message":"My preference would be to avoid these relationships and handle the joins explicitly in the db api, but I would like to get some other opinions on that.  There have been a number of times when we\u0027ve wanted to avoid joins that are coded this way and it takes extra work to do so.  But this is not a showstopper for me.\n\nIs this tested anywhere?  I\u0027m not really familiar with the relationship setup and this is different than others I see defined in Nova already.  It would be good to have a test to show that this works.","commit_id":"6e66ea38f9ef775b2f1653bdff8d9ab071598a18"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"cadbfe34aa292f2dff40bdce31078eca4ef6424d","unresolved":false,"context_lines":[{"line_number":71,"context_line":"            nullable\u003dFalse)"},{"line_number":72,"context_line":"    host \u003d Column(String(255), nullable\u003dFalse)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    cell_mapping \u003d relationship(\"CellMapping\", backref\u003d\"hosts\", lazy\u003d\u0027joined\u0027,"},{"line_number":75,"context_line":"                                innerjoin\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_98db8ded","line":74,"in_reply_to":"ba3cc151_68ba9ec2","updated":"2015-07-06 21:07:05.000000000","message":"While I agree that we\u0027ll probably always want the cell_mapping when pulling the host_mapping there have been a few times where we\u0027ve changed our minds later on something like this, and then had to explicitly unload things.\n\nI would just handle the relationship in the db methods so we can pull both models together but later have an option to just pull one.  It just keeps us from having to fight sqlalchemy later.\n\nThis is just a preference of mine though, and I\u0027m not going to hold up on this point.","commit_id":"6e66ea38f9ef775b2f1653bdff8d9ab071598a18"},{"author":{"_account_id":12548,"name":"Dheeraj Gupta","email":"dheeraj.gupta4@gmail.com","username":"dgupta"},"change_message_id":"e6f4620013ab4027689ea233994740d522ddbbd1","unresolved":false,"context_lines":[{"line_number":71,"context_line":"            nullable\u003dFalse)"},{"line_number":72,"context_line":"    host \u003d Column(String(255), nullable\u003dFalse)"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"    cell_mapping \u003d relationship(\"CellMapping\", backref\u003d\"hosts\", lazy\u003d\u0027joined\u0027,"},{"line_number":75,"context_line":"                                innerjoin\u003dTrue)"}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_68ba9ec2","line":74,"in_reply_to":"ba3cc151_cf0ecd8b","updated":"2015-07-06 09:37:05.000000000","message":"Since cell_mapping is something we\u0027ll probably always want when looking at host-mapping, I put this as an eager-loaded relationship.\n\nThis is tested, though not in a separate test, in functional tests. I\u0027ll write a test that explicitly tests the \"cell_mapping\" attribute.","commit_id":"6e66ea38f9ef775b2f1653bdff8d9ab071598a18"}],"nova/objects/host_mapping.py":[{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"f223b71b3fe12ed20e3ca3c2635b17185c9b18eb","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    fields \u003d {"},{"line_number":26,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":27,"context_line":"        \u0027host\u0027: fields.StringField(),"},{"line_number":28,"context_line":"        \u0027cell_id\u0027: fields.IntegerField(),"},{"line_number":29,"context_line":"        }"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_fd33f911","line":28,"updated":"2015-06-18 22:22:07.000000000","message":"This object isn\u0027t very useful on it\u0027s own, it\u0027s mainly a proxy for getting a cell_mapping.  So rather than having cell_id as a field it would be better to have it get a whole CellMapping object and present that as a field.","commit_id":"d5a45d1a47b8bcafcfd97157edfc554403867cee"},{"author":{"_account_id":12548,"name":"Dheeraj Gupta","email":"dheeraj.gupta4@gmail.com","username":"dgupta"},"change_message_id":"54c45ce80d3da14bb07cdcf4e0af13df855da1d3","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    fields \u003d {"},{"line_number":26,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":27,"context_line":"        \u0027host\u0027: fields.StringField(),"},{"line_number":28,"context_line":"        \u0027cell_id\u0027: fields.IntegerField(),"},{"line_number":29,"context_line":"        }"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"ba3cc151_bc540821","line":28,"in_reply_to":"fa32b979_fd33f911","updated":"2015-07-01 09:59:35.000000000","message":"Done","commit_id":"d5a45d1a47b8bcafcfd97157edfc554403867cee"},{"author":{"_account_id":12548,"name":"Dheeraj Gupta","email":"dheeraj.gupta4@gmail.com","username":"dgupta"},"change_message_id":"0e97e820ca6517e6283c5c51020eeee9c96c2c8b","unresolved":false,"context_lines":[{"line_number":25,"context_line":"    fields \u003d {"},{"line_number":26,"context_line":"        \u0027id\u0027: fields.IntegerField(read_only\u003dTrue),"},{"line_number":27,"context_line":"        \u0027host\u0027: fields.StringField(),"},{"line_number":28,"context_line":"        \u0027cell_id\u0027: fields.IntegerField(),"},{"line_number":29,"context_line":"        }"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    @staticmethod"}],"source_content_type":"text/x-python","patch_set":4,"id":"fa32b979_300af1d2","line":28,"in_reply_to":"fa32b979_fd33f911","updated":"2015-06-25 10:39:58.000000000","message":"I am working on having an ObjectField for CellMapping. But I am not sure how to populate the cell_mapping sub-object. Should it be 1) through a relationship on HostMapping table 2) adding a get_by_id method to the CellMapping object? 3) Any other way?","commit_id":"d5a45d1a47b8bcafcfd97157edfc554403867cee"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"bf972b8a55fa26ecf3e457edfb15295300139193","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        with session.begin():"},{"line_number":64,"context_line":"            db_mapping \u003d session.query("},{"line_number":65,"context_line":"                    api_models.HostMapping).filter_by("},{"line_number":66,"context_line":"                            host\u003dhost).first()"},{"line_number":67,"context_line":"            if not db_mapping:"},{"line_number":68,"context_line":"                raise exception.HostMappingNotFound(name\u003dhost)"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_2f75f9f8","line":66,"updated":"2015-07-01 21:13:08.000000000","message":"I would rather the join be done here explicitly rather than relying on the lazy join behavior in _from_db_object.","commit_id":"6e66ea38f9ef775b2f1653bdff8d9ab071598a18"},{"author":{"_account_id":12548,"name":"Dheeraj Gupta","email":"dheeraj.gupta4@gmail.com","username":"dgupta"},"change_message_id":"e6f4620013ab4027689ea233994740d522ddbbd1","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        with session.begin():"},{"line_number":64,"context_line":"            db_mapping \u003d session.query("},{"line_number":65,"context_line":"                    api_models.HostMapping).filter_by("},{"line_number":66,"context_line":"                            host\u003dhost).first()"},{"line_number":67,"context_line":"            if not db_mapping:"},{"line_number":68,"context_line":"                raise exception.HostMappingNotFound(name\u003dhost)"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_3e736cb6","line":66,"in_reply_to":"ba3cc151_2f75f9f8","updated":"2015-07-06 09:37:05.000000000","message":"As the relationship is eager-loaded, the join happens here itself (though it is implicit).\n\nDoes explicit join mean you want a session.query(HostMapping).join(CellMapping) here or just a joinedload(\"cell_mapping\")  as in http://git.openstack.org/cgit/openstack/nova/tree/nova/db/sqlalchemy/api.py#n1766?","commit_id":"6e66ea38f9ef775b2f1653bdff8d9ab071598a18"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"cadbfe34aa292f2dff40bdce31078eca4ef6424d","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        with session.begin():"},{"line_number":64,"context_line":"            db_mapping \u003d session.query("},{"line_number":65,"context_line":"                    api_models.HostMapping).filter_by("},{"line_number":66,"context_line":"                            host\u003dhost).first()"},{"line_number":67,"context_line":"            if not db_mapping:"},{"line_number":68,"context_line":"                raise exception.HostMappingNotFound(name\u003dhost)"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_58fc253e","line":66,"in_reply_to":"ba3cc151_3e736cb6","updated":"2015-07-06 21:07:05.000000000","message":"a .join(CellMapping).  I know that currently the join is implicitly done here.  My preference would be to do it explicitly instead.","commit_id":"6e66ea38f9ef775b2f1653bdff8d9ab071598a18"},{"author":{"_account_id":12548,"name":"Dheeraj Gupta","email":"dheeraj.gupta4@gmail.com","username":"dgupta"},"change_message_id":"dc566daf17c01f135d3fdafb88d549bc6a09a535","unresolved":false,"context_lines":[{"line_number":63,"context_line":"        with session.begin():"},{"line_number":64,"context_line":"            db_mapping \u003d session.query("},{"line_number":65,"context_line":"                    api_models.HostMapping).filter_by("},{"line_number":66,"context_line":"                            host\u003dhost).first()"},{"line_number":67,"context_line":"            if not db_mapping:"},{"line_number":68,"context_line":"                raise exception.HostMappingNotFound(name\u003dhost)"},{"line_number":69,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_0267bcb7","line":66,"in_reply_to":"ba3cc151_58fc253e","updated":"2015-07-08 09:06:32.000000000","message":"As per my understanding, if we add a join(CellMapping, we will have to return both HostMapping and the joined CellMapping. _from_db_object would then expect both db_host_mapping and db_cell_mapping which doesn\u0027t seem right.","commit_id":"6e66ea38f9ef775b2f1653bdff8d9ab071598a18"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"bf972b8a55fa26ecf3e457edfb15295300139193","unresolved":false,"context_lines":[{"line_number":80,"context_line":"        db_mapping \u003d api_models.HostMapping()"},{"line_number":81,"context_line":"        db_mapping.update(updates)"},{"line_number":82,"context_line":"        db_mapping.save(session)"},{"line_number":83,"context_line":"        # Need to load cell_mapping attribute"},{"line_number":84,"context_line":"        session.refresh(db_mapping)"},{"line_number":85,"context_line":"        return db_mapping"},{"line_number":86,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"ba3cc151_efbdd168","line":83,"updated":"2015-07-01 21:13:08.000000000","message":"You can rely on lazy loading to grab it if necessary.  But you\u0027ll need to implement the obj_load_attr method like http://git.openstack.org/cgit/openstack/nova/tree/nova/objects/instance.py#n1022","commit_id":"6e66ea38f9ef775b2f1653bdff8d9ab071598a18"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"84fc95bbcd94141b5f30ac5775a9b1393cafcd7d","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    def _from_db_object(context, host_mapping, db_host_mapping):"},{"line_number":62,"context_line":"        for key in host_mapping.fields:"},{"line_number":63,"context_line":"            if key \u003d\u003d \"cell_mapping\":"},{"line_number":64,"context_line":"                continue"},{"line_number":65,"context_line":"            setattr(host_mapping, key, db_host_mapping.get(key))"},{"line_number":66,"context_line":"        host_mapping.obj_reset_changes()"},{"line_number":67,"context_line":"        host_mapping._context \u003d context"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_b357d9ff","line":64,"updated":"2015-07-09 20:53:19.000000000","message":"This should check if db_host_mapping has cell_mapping defined and if so load it.","commit_id":"65a2823e61bceace7b57ed2368cb6d02cb3e523a"},{"author":{"_account_id":12548,"name":"Dheeraj Gupta","email":"dheeraj.gupta4@gmail.com","username":"dgupta"},"change_message_id":"e51dc9212f14aa14ff56249038394da116565180","unresolved":false,"context_lines":[{"line_number":61,"context_line":"    def _from_db_object(context, host_mapping, db_host_mapping):"},{"line_number":62,"context_line":"        for key in host_mapping.fields:"},{"line_number":63,"context_line":"            if key \u003d\u003d \"cell_mapping\":"},{"line_number":64,"context_line":"                continue"},{"line_number":65,"context_line":"            setattr(host_mapping, key, db_host_mapping.get(key))"},{"line_number":66,"context_line":"        host_mapping.obj_reset_changes()"},{"line_number":67,"context_line":"        host_mapping._context \u003d context"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_1bec8c91","line":64,"in_reply_to":"ba3cc151_b357d9ff","updated":"2015-07-10 07:38:57.000000000","message":"Done","commit_id":"65a2823e61bceace7b57ed2368cb6d02cb3e523a"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"84fc95bbcd94141b5f30ac5775a9b1393cafcd7d","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        with session.begin():"},{"line_number":75,"context_line":"            db_mapping \u003d (session.query(api_models.HostMapping)"},{"line_number":76,"context_line":"                          .filter_by(host\u003dhost)).first()"},{"line_number":77,"context_line":"            if not db_mapping:"},{"line_number":78,"context_line":"                raise exception.HostMappingNotFound(name\u003dhost)"},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_73dab179","line":76,"updated":"2015-07-09 20:53:19.000000000","message":"If you join the cell_mapping here the it will be available for _from_db_object().  Since get_by_host is going to be the primary method for creating this object if it\u0027s optimized for loading both host and cell_mapping we should be good.","commit_id":"65a2823e61bceace7b57ed2368cb6d02cb3e523a"},{"author":{"_account_id":12548,"name":"Dheeraj Gupta","email":"dheeraj.gupta4@gmail.com","username":"dgupta"},"change_message_id":"e51dc9212f14aa14ff56249038394da116565180","unresolved":false,"context_lines":[{"line_number":73,"context_line":""},{"line_number":74,"context_line":"        with session.begin():"},{"line_number":75,"context_line":"            db_mapping \u003d (session.query(api_models.HostMapping)"},{"line_number":76,"context_line":"                          .filter_by(host\u003dhost)).first()"},{"line_number":77,"context_line":"            if not db_mapping:"},{"line_number":78,"context_line":"                raise exception.HostMappingNotFound(name\u003dhost)"},{"line_number":79,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba3cc151_bbd6c0c1","line":76,"in_reply_to":"ba3cc151_73dab179","updated":"2015-07-10 07:38:57.000000000","message":"I have now made a join and added the CellMapping to entities being queried. The CellMapping is then stashed in a \"cell_mapping\" attribute of the HostMapping db object and the HostMapping db object is returned","commit_id":"65a2823e61bceace7b57ed2368cb6d02cb3e523a"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"26a61aa4ffc9332d1b2cedd95b5e7be12f6fc853","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"        with session.begin():"},{"line_number":82,"context_line":"            db_mapping \u003d (session.query(api_models.HostMapping)"},{"line_number":83,"context_line":"                          .join(api_models.CellMapping)"},{"line_number":84,"context_line":"                          .with_entities(api_models.HostMapping,"},{"line_number":85,"context_line":"                                         api_models.CellMapping)"},{"line_number":86,"context_line":"                          .filter(api_models.HostMapping.host \u003d\u003d host)).first()"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_628636dd","line":83,"updated":"2015-07-10 15:15:35.000000000","message":"There\u0027s a two argument form of join which allows you to specify an ON clause, so \".join(api_models.CellMapping, api_model.HostMapping.cell_id \u003d\u003dapi_models.CellMapping.id)\" should work.  And it should work for _get_cell_mapping as well.\n\nSqlalchemy does really want a relationship defined here though.  I didn\u0027t realize how much it would take to work around that.  I\u0027m going to backtrack and say that while I still prefer to avoid it I\u0027m not going to hold this up if you would prefer to go that route.","commit_id":"d8049b4813edd6ace0d8e2520a64856aa57318cd"},{"author":{"_account_id":12548,"name":"Dheeraj Gupta","email":"dheeraj.gupta4@gmail.com","username":"dgupta"},"change_message_id":"c54830c0c3aff769a0cc4616be6fb91797a4cb7b","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"        with session.begin():"},{"line_number":82,"context_line":"            db_mapping \u003d (session.query(api_models.HostMapping)"},{"line_number":83,"context_line":"                          .join(api_models.CellMapping)"},{"line_number":84,"context_line":"                          .with_entities(api_models.HostMapping,"},{"line_number":85,"context_line":"                                         api_models.CellMapping)"},{"line_number":86,"context_line":"                          .filter(api_models.HostMapping.host \u003d\u003d host)).first()"}],"source_content_type":"text/x-python","patch_set":7,"id":"ba3cc151_f149aeb8","line":83,"in_reply_to":"ba3cc151_628636dd","updated":"2015-07-10 16:41:16.000000000","message":"AFAIK join clause is determined by ForeignKey constraint and not the relationship. So it may be redundant in this case. Also if join condition cannot be determined for a join, SQLAlchemy will throw an error rather than do a \"blind\" join. Since functional tests pass, I assumed the ON clause is automatically determined by Sqlalchemy","commit_id":"d8049b4813edd6ace0d8e2520a64856aa57318cd"},{"author":{"_account_id":5441,"name":"Andrew Laski","email":"andrew@lascii.com","username":"alaski"},"change_message_id":"aa5956552f6514cb1b3d062664af5c0abd036a35","unresolved":false,"context_lines":[{"line_number":80,"context_line":""},{"line_number":81,"context_line":"        with session.begin():"},{"line_number":82,"context_line":"            db_mapping \u003d (session.query(api_models.HostMapping)"},{"line_number":83,"context_line":"                          .join(api_models.CellMapping)"},{"line_number":84,"context_line":"                          .with_entities(api_models.HostMapping,"},{"line_number":85,"context_line":"                                         api_models.CellMapping)"},{"line_number":86,"context_line":"                          .filter(api_models.HostMapping.host \u003d\u003d host)).first()"}],"source_content_type":"text/x-python","patch_set":7,"id":"9a41bdd9_b6e8d9e3","line":83,"in_reply_to":"ba3cc151_f149aeb8","updated":"2015-07-13 14:54:17.000000000","message":"I just took a look at the SQL emitted by sqlalchemy during this query and it contains the proper ON clause.  So my concern is invalid.","commit_id":"d8049b4813edd6ace0d8e2520a64856aa57318cd"}]}