)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"574d37adada096cdaf00cf1fb717ad721fbc5960","unresolved":false,"context_lines":[{"line_number":16,"context_line":"well as a cached root tree ID, it became obvious that using"},{"line_number":17,"context_line":"the SQLAlchemy ORM modeling to generate the required queries for"},{"line_number":18,"context_line":"various methods related to resource providers was resulting in awkward"},{"line_number":19,"context_line":"and hard-to-reason-about code. Even using the recommend handling of"},{"line_number":20,"context_line":"self-referential tables [1] for adjacency list modeling, the way the"},{"line_number":21,"context_line":"session and querying handling was done in the resource_provider.py"},{"line_number":22,"context_line":"module lead to a number of lazy-loading problems and inactive session"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7f515b1d_eaa4a3bb","line":19,"range":{"start_line":19,"start_character":46,"end_line":19,"end_character":55},"updated":"2017-10-04 14:02:51.000000000","message":"recomended","commit_id":"91dec3a4d6de50020f53ab1db39bb5741b47f744"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"65d5b73793c4df084ec1753580a1e707af96b777","unresolved":false,"context_lines":[{"line_number":16,"context_line":"well as a cached root tree ID, it became obvious that using"},{"line_number":17,"context_line":"the SQLAlchemy ORM modeling to generate the required queries for"},{"line_number":18,"context_line":"various methods related to resource providers was resulting in awkward"},{"line_number":19,"context_line":"and hard-to-reason-about code. Even using the recommend handling of"},{"line_number":20,"context_line":"self-referential tables [1] for adjacency list modeling, the way the"},{"line_number":21,"context_line":"session and querying handling was done in the resource_provider.py"},{"line_number":22,"context_line":"module lead to a number of lazy-loading problems and inactive session"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7f515b1d_032ea4a4","line":19,"range":{"start_line":19,"start_character":46,"end_line":19,"end_character":55},"in_reply_to":"7f515b1d_c0fe5b6a","updated":"2017-10-05 14:29:21.000000000","message":"Done","commit_id":"91dec3a4d6de50020f53ab1db39bb5741b47f744"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0f5c042aa2aea9a1b5a348c9e9f1d2f50bac2699","unresolved":false,"context_lines":[{"line_number":16,"context_line":"well as a cached root tree ID, it became obvious that using"},{"line_number":17,"context_line":"the SQLAlchemy ORM modeling to generate the required queries for"},{"line_number":18,"context_line":"various methods related to resource providers was resulting in awkward"},{"line_number":19,"context_line":"and hard-to-reason-about code. Even using the recommend handling of"},{"line_number":20,"context_line":"self-referential tables [1] for adjacency list modeling, the way the"},{"line_number":21,"context_line":"session and querying handling was done in the resource_provider.py"},{"line_number":22,"context_line":"module lead to a number of lazy-loading problems and inactive session"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7f515b1d_c0fe5b6a","line":19,"range":{"start_line":19,"start_character":46,"end_line":19,"end_character":55},"in_reply_to":"7f515b1d_eaa4a3bb","updated":"2017-10-04 21:27:44.000000000","message":"Or even recommended","commit_id":"91dec3a4d6de50020f53ab1db39bb5741b47f744"},{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0f5c042aa2aea9a1b5a348c9e9f1d2f50bac2699","unresolved":false,"context_lines":[{"line_number":19,"context_line":"and hard-to-reason-about code. Even using the recommend handling of"},{"line_number":20,"context_line":"self-referential tables [1] for adjacency list modeling, the way the"},{"line_number":21,"context_line":"session and querying handling was done in the resource_provider.py"},{"line_number":22,"context_line":"module lead to a number of lazy-loading problems and inactive session"},{"line_number":23,"context_line":"errors."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"In this starter patch, we tackle the ResourceProvider.get_by_uuid()"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7f515b1d_042feef6","line":22,"range":{"start_line":22,"start_character":7,"end_line":22,"end_character":11},"updated":"2017-10-04 21:27:44.000000000","message":"led","commit_id":"91dec3a4d6de50020f53ab1db39bb5741b47f744"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"65d5b73793c4df084ec1753580a1e707af96b777","unresolved":false,"context_lines":[{"line_number":19,"context_line":"and hard-to-reason-about code. Even using the recommend handling of"},{"line_number":20,"context_line":"self-referential tables [1] for adjacency list modeling, the way the"},{"line_number":21,"context_line":"session and querying handling was done in the resource_provider.py"},{"line_number":22,"context_line":"module lead to a number of lazy-loading problems and inactive session"},{"line_number":23,"context_line":"errors."},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"In this starter patch, we tackle the ResourceProvider.get_by_uuid()"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"7f515b1d_8313b466","line":22,"range":{"start_line":22,"start_character":7,"end_line":22,"end_character":11},"in_reply_to":"7f515b1d_042feef6","updated":"2017-10-05 14:29:21.000000000","message":"Done","commit_id":"91dec3a4d6de50020f53ab1db39bb5741b47f744"}],"nova/objects/resource_provider.py":[{"author":{"_account_id":14070,"name":"Eric Fried","email":"openstack@fried.cc","username":"efried"},"change_message_id":"0f5c042aa2aea9a1b5a348c9e9f1d2f50bac2699","unresolved":false,"context_lines":[{"line_number":448,"context_line":"        self._update_in_db(self._context, self.id, updates)"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"    @classmethod"},{"line_number":451,"context_line":"    def get_by_uuid(cls, context, uuid):"},{"line_number":452,"context_line":"        rp_rec \u003d _get_provider_by_uuid(context, uuid)"},{"line_number":453,"context_line":"        return cls._from_db_object(context, cls(), rp_rec)"},{"line_number":454,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_001353ba","line":451,"range":{"start_line":451,"start_character":8,"end_line":451,"end_character":19},"updated":"2017-10-04 21:27:44.000000000","message":"Good time to add a docstring, to include the fact that this can raise NotFound.","commit_id":"91dec3a4d6de50020f53ab1db39bb5741b47f744"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"65d5b73793c4df084ec1753580a1e707af96b777","unresolved":false,"context_lines":[{"line_number":448,"context_line":"        self._update_in_db(self._context, self.id, updates)"},{"line_number":449,"context_line":""},{"line_number":450,"context_line":"    @classmethod"},{"line_number":451,"context_line":"    def get_by_uuid(cls, context, uuid):"},{"line_number":452,"context_line":"        rp_rec \u003d _get_provider_by_uuid(context, uuid)"},{"line_number":453,"context_line":"        return cls._from_db_object(context, cls(), rp_rec)"},{"line_number":454,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"7f515b1d_a34eb093","line":451,"range":{"start_line":451,"start_character":8,"end_line":451,"end_character":19},"in_reply_to":"7f515b1d_001353ba","updated":"2017-10-05 14:29:21.000000000","message":"Done","commit_id":"91dec3a4d6de50020f53ab1db39bb5741b47f744"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"974239ecd181e46faff4f3203530c06a9b12c7ff","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    return exceeded"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"@db_api.api_context_manager.reader"},{"line_number":392,"context_line":"def _get_provider_by_uuid(context, uuid):"},{"line_number":393,"context_line":"    \"\"\"Given a UUID, return a dict of information about the resource provider"},{"line_number":394,"context_line":"    from the database."}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_1ed5b643","line":391,"updated":"2017-10-06 16:14:16.000000000","message":"any reason why you can\u0027t use a staticmethod deco here?\n\nhttps://bugs.launchpad.net/oslo.db/+bug/1520195 is fixed since a very long time, and I\u0027d love to see the DB API methods be in the related class","commit_id":"c47c07c3a15dd0c50e900c827e0775f97fb11134"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6d42c74d6790f0e157c0b3d23d5c4d3ebb3656fb","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    return exceeded"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"@db_api.api_context_manager.reader"},{"line_number":392,"context_line":"def _get_provider_by_uuid(context, uuid):"},{"line_number":393,"context_line":"    \"\"\"Given a UUID, return a dict of information about the resource provider"},{"line_number":394,"context_line":"    from the database."}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_662e5ca6","line":391,"in_reply_to":"7f515b1d_1ed5b643","updated":"2017-10-06 18:50:27.000000000","message":"I hate the staticmethods and have moved all these staticmethods out from the classes and put them together with all the module-level functions that only use the SQLAlchemy core expression API.","commit_id":"c47c07c3a15dd0c50e900c827e0775f97fb11134"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"66abe1c798e1059088f47706f90e7ddfa1d2319c","unresolved":false,"context_lines":[{"line_number":388,"context_line":"    return exceeded"},{"line_number":389,"context_line":""},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"@db_api.api_context_manager.reader"},{"line_number":392,"context_line":"def _get_provider_by_uuid(context, uuid):"},{"line_number":393,"context_line":"    \"\"\"Given a UUID, return a dict of information about the resource provider"},{"line_number":394,"context_line":"    from the database."}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_f97acd74","line":391,"in_reply_to":"7f515b1d_662e5ca6","updated":"2017-10-09 09:14:49.000000000","message":"So, the CRUD methods for each table would be at the module-level instead of being in the same class ? Well, if you seriously think of the existing huuuuuuuge DB API we have (and how Python is bad with module-level functions), do you still think it\u0027s a nice pattern ? \nCan I ask why you hate the staticmethod Python design pattern then ?","commit_id":"c47c07c3a15dd0c50e900c827e0775f97fb11134"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"974239ecd181e46faff4f3203530c06a9b12c7ff","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        rpt.c.id,"},{"line_number":403,"context_line":"        rpt.c.uuid,"},{"line_number":404,"context_line":"        rpt.c.name,"},{"line_number":405,"context_line":"        rpt.c.generation,"},{"line_number":406,"context_line":"    ]"},{"line_number":407,"context_line":"    sel \u003d sa.select(cols).where(rpt.c.uuid \u003d\u003d uuid)"},{"line_number":408,"context_line":"    res \u003d conn.execute(sel).fetchone()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_dedf1e38","line":405,"updated":"2017-10-06 16:14:16.000000000","message":"the problem I see with passing the columns we need is that if we create a new table col, it needs to modify all the methods looking at the table, while the SQLA ORM just creates a new SQLA object with all the fields set.\n\nCould we maybe do the same and create a (or use the existing model) ResourceProvider python object (or just a namedtuple) that each DB method would use it?","commit_id":"c47c07c3a15dd0c50e900c827e0775f97fb11134"},{"author":{"_account_id":7166,"name":"Sylvain Bauza","email":"sbauza@redhat.com","username":"sbauza"},"change_message_id":"66abe1c798e1059088f47706f90e7ddfa1d2319c","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        rpt.c.id,"},{"line_number":403,"context_line":"        rpt.c.uuid,"},{"line_number":404,"context_line":"        rpt.c.name,"},{"line_number":405,"context_line":"        rpt.c.generation,"},{"line_number":406,"context_line":"    ]"},{"line_number":407,"context_line":"    sel \u003d sa.select(cols).where(rpt.c.uuid \u003d\u003d uuid)"},{"line_number":408,"context_line":"    res \u003d conn.execute(sel).fetchone()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_f9a1adf8","line":405,"in_reply_to":"7f515b1d_a9dc2774","updated":"2017-10-09 09:14:49.000000000","message":"Jay, I\u0027m not against your blueprint. I\u0027m just asking for providing a way to use some python object instead of saying which columns to select *for each SQL query*\n\nLike I said, say we create a new column for the ResourceProvider table, it would need to add it for the above L405, right?","commit_id":"c47c07c3a15dd0c50e900c827e0775f97fb11134"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6d42c74d6790f0e157c0b3d23d5c4d3ebb3656fb","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        rpt.c.id,"},{"line_number":403,"context_line":"        rpt.c.uuid,"},{"line_number":404,"context_line":"        rpt.c.name,"},{"line_number":405,"context_line":"        rpt.c.generation,"},{"line_number":406,"context_line":"    ]"},{"line_number":407,"context_line":"    sel \u003d sa.select(cols).where(rpt.c.uuid \u003d\u003d uuid)"},{"line_number":408,"context_line":"    res \u003d conn.execute(sel).fetchone()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_a9dc2774","line":405,"in_reply_to":"7f515b1d_dedf1e38","updated":"2017-10-06 18:50:27.000000000","message":"The ORM is slow, can produce inefficient and overly-verbose SQL and it hides things -- like which columns are actually being projected into the end result. This is why this series is removing all querying via the ORM.\n\nA good example of how the inefficient SQL generated by ORM-based queries is when you add a root_provider_id and parent_provider_id field to the ResourceProvider model, add backref relationships to represent the self-referential model and tree referential model and then try to do a query like this:\n\n rp \u003d ctx.session.query(models.ResourceProvider).filter(models.ResourceProvider.root_provider.uuid \u003d\u003d root_uuid).all()\n // Try to do something with the rp.parent_provider field...\n\n You will notice that the SQL generated for the above simple-looking query is a complete mess and overly verbose. If you specify that parent_provider should be eagerly-loaded in the relationship() definition on the ResourceProvider ORM model definition, then you\u0027ll be making a LEFT JOIN from resource_providers to resource_providers every time you use the api_models.ResourceProvider model in any query. If you *don\u0027t* use eager loading, then when you do need the parent information, you\u0027ll be executing multiple queries.\n\nUsing the core expression API and being explicit about the SQL that is constructed means we produce the most efficient SQL that is necessary to solve the specific query we\u0027re trying to do.","commit_id":"c47c07c3a15dd0c50e900c827e0775f97fb11134"},{"author":{"_account_id":7,"name":"Jay Pipes","email":"jaypipes@gmail.com","username":"jaypipes"},"change_message_id":"6569074a4cc661469f64632d4829792985db439e","unresolved":false,"context_lines":[{"line_number":402,"context_line":"        rpt.c.id,"},{"line_number":403,"context_line":"        rpt.c.uuid,"},{"line_number":404,"context_line":"        rpt.c.name,"},{"line_number":405,"context_line":"        rpt.c.generation,"},{"line_number":406,"context_line":"    ]"},{"line_number":407,"context_line":"    sel \u003d sa.select(cols).where(rpt.c.uuid \u003d\u003d uuid)"},{"line_number":408,"context_line":"    res \u003d conn.execute(sel).fetchone()"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f515b1d_3f6962f8","line":405,"in_reply_to":"7f515b1d_f9a1adf8","updated":"2017-10-09 12:20:10.000000000","message":"Only if the callers of the above function need access to that field.","commit_id":"c47c07c3a15dd0c50e900c827e0775f97fb11134"}]}