)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":33582,"name":"Mark Powers","email":"markpowers@uchicago.edu","username":"markpowers"},"change_message_id":"69e0db1cd25e15847d95836cd08e1e7b38924092","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"863bde52_d725e1c5","updated":"2024-09-26 16:40:03.000000000","message":"Hi John, I was just about to start working on a similar issue :)\n\nIs there any issue with moving the enforcement `try/except` one block further down (after the `try/except` calling `_create_reservation)?\n\nWe are using this to \"charge\" users per-reservation, and so it would be useful to have the reservation ID field initialized before `check_create`, which is done by `_create_reservation()`.","commit_id":"c93015fa993a49cc2395a14eaac31e82c38f2b6c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"fbc55241cf1e356d06d86a1b74c58da470ed105e","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"ecf5bdf8_ffaae3bb","in_reply_to":"2caf4809_bdd069f7","updated":"2024-09-27 17:00:17.000000000","message":"More context... plan A was doing basically a separate check and commit call.\n\nI think we probably need this for my patch to add --dry run. But I think we can get away with this in the short term, if we have the correct id, but add some extra on_end calls.\n\nAlthough it sounds like you have some good cases where that isn\u0027t the case?","commit_id":"c93015fa993a49cc2395a14eaac31e82c38f2b6c"},{"author":{"_account_id":782,"name":"John Garbutt","email":"john@johngarbutt.com","username":"johngarbutt"},"change_message_id":"b41ced668bcc6285f74331e63a98f25c49979db0","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2caf4809_bdd069f7","in_reply_to":"863bde52_d725e1c5","updated":"2024-09-27 16:58:05.000000000","message":"Ah, so we actually called that \"commit\" instead of \"check\". The problem with moving it a bit further down is you actually need to make all the cleanup logic work.\n\nThis is my compromise, make sure the id is sent, so at least you can resolve any differences.\n\nFWIW, we are charging uses via this thing, based on the flavor resources (similar to the placement ones):\nhttps://github.com/stackhpc/coral-credits","commit_id":"c93015fa993a49cc2395a14eaac31e82c38f2b6c"}],"blazar/manager/service.py":[{"author":{"_account_id":33582,"name":"Mark Powers","email":"markpowers@uchicago.edu","username":"markpowers"},"change_message_id":"605d530200eed0b144b5322e7a496d9e461033c9","unresolved":true,"context_lines":[{"line_number":430,"context_line":"                    self.enforcement.check_create("},{"line_number":431,"context_line":"                        context.current(), lease_values, reservations,"},{"line_number":432,"context_line":"                        allocations, resource_requests)"},{"line_number":433,"context_line":"                except common_ex.NotAuthorized as e:"},{"line_number":434,"context_line":"                    db_api.lease_destroy(lease_id)"},{"line_number":435,"context_line":"                    LOG.error(\"Enforcement checks failed. %s\", str(e))"},{"line_number":436,"context_line":"                    raise common_ex.NotAuthorized(e)"}],"source_content_type":"text/x-python","patch_set":2,"id":"eab0ea2d_15420a1c","line":433,"updated":"2024-11-01 15:27:58.000000000","message":"I found an issue testing this in our fork. External enforcement `check_create` does not raise an instance of `NotAuthorized`, instead it raises a generic `BlazarException` subclass: https://opendev.org/openstack/blazar/src/branch/stable/2024.2/blazar/enforcement/exceptions.py#L26\nhttps://opendev.org/openstack/blazar/src/branch/master/blazar/enforcement/filters/external_service_filter.py#L155\n\nSo this `except` block won\u0027t catch the exception and therefore the lease won\u0027t get deleted. I think the proper fix should be updating `ExternalServiceFilterException` to be a subclass of `NotAuthorized`.","commit_id":"728ef497f33166a40abe25a32087f3b457a26ca1"}]}