)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":3153,"name":"Emilien Macchi","email":"emilien@redhat.com","username":"emilienm"},"change_message_id":"dfe1bfe85d8fea4f0d78cc76a2b80abacc606e18","unresolved":true,"context_lines":[{"line_number":19,"context_line":"do strict type checking."},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"TODO:"},{"line_number":22,"context_line":"- File and reference a bug"},{"line_number":23,"context_line":"- Tests"},{"line_number":24,"context_line":""},{"line_number":25,"context_line":"This patch has had manual testing only."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"b8181e99_4a65f769","line":22,"updated":"2021-03-11 02:51:52.000000000","message":"https://bugs.launchpad.net/neutron/+bug/1918565","commit_id":"3d7e2f8e8a50ae7ff2ab2c74a25e49106f9c3a8b"}],"neutron/objects/quota.py":[{"author":{"_account_id":1131,"name":"Brian Haley","email":"haleyb.dev@gmail.com","username":"brian-haley"},"change_message_id":"8f44ebc6a7f462f73072ef35f3f421e8f7404f3a","unresolved":true,"context_lines":[{"line_number":93,"context_line":"            models.Reservation.expiration,"},{"line_number":94,"context_line":"            sql.func.cast("},{"line_number":95,"context_line":"                sql.func.sum(models.ResourceDelta.amount),"},{"line_number":96,"context_line":"                sqltypes.Integer)).join("},{"line_number":97,"context_line":"            models.Reservation)"},{"line_number":98,"context_line":"        if expired:"},{"line_number":99,"context_line":"            exp_expr \u003d (models.Reservation.expiration \u003c now)"}],"source_content_type":"text/x-python","patch_set":1,"id":"0dec49b0_7d2a5319","line":96,"updated":"2021-03-11 04:04:16.000000000","message":"I guess this should work, or an int(sql.func.sum(...)), or even looking at the caller, neutron.quota.resource.count_reserved(), it could have forced the int() on return as well.  Too many ways to fix it.","commit_id":"3d7e2f8e8a50ae7ff2ab2c74a25e49106f9c3a8b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"8aa4cfafcd02c0654cea6dd8e7b1890daf1e861e","unresolved":true,"context_lines":[{"line_number":93,"context_line":"            models.Reservation.expiration,"},{"line_number":94,"context_line":"            sql.func.cast("},{"line_number":95,"context_line":"                sql.func.sum(models.ResourceDelta.amount),"},{"line_number":96,"context_line":"                sqltypes.Integer)).join("},{"line_number":97,"context_line":"            models.Reservation)"},{"line_number":98,"context_line":"        if expired:"},{"line_number":99,"context_line":"            exp_expr \u003d (models.Reservation.expiration \u003c now)"}],"source_content_type":"text/x-python","patch_set":1,"id":"522fbde7_5400e302","line":96,"in_reply_to":"0dec49b0_7d2a5319","updated":"2021-03-11 08:48:14.000000000","message":"I don\u0027t think you can use int() here because it\u0027s in a SQLAlchemy expression. We could probably use int() later because it appears that a Decimal casts to a string which we can then cast to an int.\n\nI thought it was safer to do it here because:\n1. The returned type is defined by the database driver, so we don\u0027t know for sure what it is or what its interface is to cast it. A SQLA cast abstracts us from that.\n2. Passing around a random database-specific type which folks will probably assume is an int felt like a bit of a landmine.\n\nI\u0027ll hopefully write some tests later which should make it easier to play with the above if we\u0027re interested. I\u0027m slightly nervous that the tests might involve SQLite and return a different type, which would be annoying.","commit_id":"3d7e2f8e8a50ae7ff2ab2c74a25e49106f9c3a8b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"b28f494c921b33f319611559e42f643312e9897e","unresolved":true,"context_lines":[{"line_number":93,"context_line":"            models.Reservation.expiration,"},{"line_number":94,"context_line":"            sql.func.cast("},{"line_number":95,"context_line":"                sql.func.sum(models.ResourceDelta.amount),"},{"line_number":96,"context_line":"                sqltypes.Integer)).join("},{"line_number":97,"context_line":"            models.Reservation)"},{"line_number":98,"context_line":"        if expired:"},{"line_number":99,"context_line":"            exp_expr \u003d (models.Reservation.expiration \u003c now)"}],"source_content_type":"text/x-python","patch_set":1,"id":"b45a07d0_e217bb01","line":96,"in_reply_to":"2d2eb1e5_39ef46d0","updated":"2021-03-15 11:18:33.000000000","message":"That\u0027s actually already covered in the quota db tests[1] because get_reservations_for_resources() is just a wrapper for get_total_reservations_map()[2]. I\u0027d already seen these tests, but I was assuming you\u0027d want a test that failed without this change, but passed with the change, and for that we need MySQL.\n\nHowever, I\u0027m more than happy to go with the existing test! I think it\u0027s justified because:\n\n* The existing test demonstrates that the change doesn\u0027t regress on SQLite\n* The existing test already implicitly asserts the type of the returned value\n* Manual testing demonstrates that the change fixes the issue on MySQL\n* Tempest testing demonstrates that the change doesn\u0027t regress MySQL in general\n* The lack of fine-grained unit/functional testing on MySQL isn\u0027t confined to this issue and can reasonably be addressed separately\n\nShall I just update the commit message and we\u0027re good to go?\n\n[1] https://opendev.org/openstack/neutron/src/commit/5f6fbcb155e19d0a46cef0e7bbfee553f0472ef3/neutron/tests/unit/db/quota/test_api.py#L242-L255\n[2] https://opendev.org/openstack/neutron/src/commit/5f6fbcb155e19d0a46cef0e7bbfee553f0472ef3/neutron/db/quota/api.py#L217-L232","commit_id":"3d7e2f8e8a50ae7ff2ab2c74a25e49106f9c3a8b"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"ae349eb65ea1a7951907b5bc13e6f29e3ea4bc82","unresolved":true,"context_lines":[{"line_number":93,"context_line":"            models.Reservation.expiration,"},{"line_number":94,"context_line":"            sql.func.cast("},{"line_number":95,"context_line":"                sql.func.sum(models.ResourceDelta.amount),"},{"line_number":96,"context_line":"                sqltypes.Integer)).join("},{"line_number":97,"context_line":"            models.Reservation)"},{"line_number":98,"context_line":"        if expired:"},{"line_number":99,"context_line":"            exp_expr \u003d (models.Reservation.expiration \u003c now)"}],"source_content_type":"text/x-python","patch_set":1,"id":"c839c207_2bde2fa8","line":96,"in_reply_to":"522fbde7_5400e302","updated":"2021-03-11 14:50:01.000000000","message":"IMO it\u0027s good to do it like it\u0027s here but I\u0027m definitely not SQLAlchemy guy 😊\nRegarding tests I think that maybe fullstack test could be the best way to test it as it would involve \"whole neutron machinery\" and mysql db. If You want I can help You write such test.","commit_id":"3d7e2f8e8a50ae7ff2ab2c74a25e49106f9c3a8b"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"b33f00d459b50f28f8a63b5c558bcdda18be6ae9","unresolved":true,"context_lines":[{"line_number":93,"context_line":"            models.Reservation.expiration,"},{"line_number":94,"context_line":"            sql.func.cast("},{"line_number":95,"context_line":"                sql.func.sum(models.ResourceDelta.amount),"},{"line_number":96,"context_line":"                sqltypes.Integer)).join("},{"line_number":97,"context_line":"            models.Reservation)"},{"line_number":98,"context_line":"        if expired:"},{"line_number":99,"context_line":"            exp_expr \u003d (models.Reservation.expiration \u003c now)"}],"source_content_type":"text/x-python","patch_set":1,"id":"2d2eb1e5_39ef46d0","line":96,"in_reply_to":"7a605944_04bb7f5f","updated":"2021-03-12 16:42:35.000000000","message":"Hi Matthew. To test this change you just need to create a unit test using the SqlTestCase class. Actually there is a class testing this OVO: ReservationDbObjectTestCase\n\nHere [1] you have an example of how to test this change: create a Reservation object, create a ResourceDelta (with a number of reserved resources) and execute the query (calling to the method \"get_total_reservations_map\"). The result of this method should contain a dictionary with the resource and the amount reserved (AS AN INTEGER).\n\n[1]http://paste.openstack.org/show/803518/","commit_id":"3d7e2f8e8a50ae7ff2ab2c74a25e49106f9c3a8b"},{"author":{"_account_id":11975,"name":"Slawek Kaplonski","email":"skaplons@redhat.com","username":"slaweq"},"change_message_id":"fe786dbc77da4be8f2fc01fd03daf21d0ac98155","unresolved":true,"context_lines":[{"line_number":93,"context_line":"            models.Reservation.expiration,"},{"line_number":94,"context_line":"            sql.func.cast("},{"line_number":95,"context_line":"                sql.func.sum(models.ResourceDelta.amount),"},{"line_number":96,"context_line":"                sqltypes.Integer)).join("},{"line_number":97,"context_line":"            models.Reservation)"},{"line_number":98,"context_line":"        if expired:"},{"line_number":99,"context_line":"            exp_expr \u003d (models.Reservation.expiration \u003c now)"}],"source_content_type":"text/x-python","patch_set":1,"id":"747ded24_4b411914","line":96,"in_reply_to":"b45a07d0_e217bb01","updated":"2021-03-15 12:05:38.000000000","message":"IMO yes, You can update commit message and we should be good to go. I was checking fullstack framework today but it seems that is will not be easy to test it there as we can\u0027t be sure that resources are under creation really (and we can\u0027t mock things there).","commit_id":"3d7e2f8e8a50ae7ff2ab2c74a25e49106f9c3a8b"},{"author":{"_account_id":9555,"name":"Matthew Booth","email":"mbooth@redhat.com","username":"MatthewBooth"},"change_message_id":"9cdbde7d18a7904ddc426ddd5d9d6562afbbe580","unresolved":true,"context_lines":[{"line_number":93,"context_line":"            models.Reservation.expiration,"},{"line_number":94,"context_line":"            sql.func.cast("},{"line_number":95,"context_line":"                sql.func.sum(models.ResourceDelta.amount),"},{"line_number":96,"context_line":"                sqltypes.Integer)).join("},{"line_number":97,"context_line":"            models.Reservation)"},{"line_number":98,"context_line":"        if expired:"},{"line_number":99,"context_line":"            exp_expr \u003d (models.Reservation.expiration \u003c now)"}],"source_content_type":"text/x-python","patch_set":1,"id":"7a605944_04bb7f5f","line":96,"in_reply_to":"c839c207_2bde2fa8","updated":"2021-03-11 15:41:05.000000000","message":"Yes please. Are you thinking of something in functional, or tempest?\n\nIf functional, are there any existing tests which run MySQL? If not then this is likely to be a mini-project in itself :) This would be ideal, though, because then it would be very simple to create a reservation to manually trigger the bug.\n\nIf tempest, can you think of a sequence of api calls which would reliably create a reservation on any resource for a well-defined period of time? If you can think of anything I can probably get going with this, too.","commit_id":"3d7e2f8e8a50ae7ff2ab2c74a25e49106f9c3a8b"}]}