)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"0bd9d05d1291ed703f35fcb885fa5b835b3e00d7","unresolved":false,"context_lines":[{"line_number":12,"context_line":"then the function which modifies ACLS is called with the former attribute,"},{"line_number":13,"context_line":"in order to keep ACLS."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Closes Bug : #1584076"},{"line_number":16,"context_line":""},{"line_number":17,"context_line":"Change-Id: I21e9deb9105faee87500998eff47c6faa1d02914"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":1,"id":"bab6814e_5ee1e4cf","line":15,"range":{"start_line":15,"start_character":0,"end_line":15,"end_character":21},"updated":"2016-05-24 17:37:15.000000000","message":"Don\u0027t use the \u0027#\u0027 character.\n\nSyntax is Closes-Bug: 1584076\n\nThat way, the bug is auto linked in the gerrit commit with LP.","commit_id":"1ec4cd57acc4b4833bcd39037566041f405fd67e"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"3946ac0ef23d5d120a61b084c4ff6ddafbc3e35b","unresolved":false,"context_lines":[{"line_number":7,"context_line":"Fix bug Swift ACL which disappears on Glance v1 images"},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"Currently, when we make an update on a public image, Swift ACL disappears."},{"line_number":10,"context_line":"This patch allows to set ACL well when an update on a Glance image is made."},{"line_number":11,"context_line":"If an update is made on a Glance image without changing the public attribute,"},{"line_number":12,"context_line":"then the function which modifies ACLS is called with the former attribute,"},{"line_number":13,"context_line":"in order to keep ACLS."},{"line_number":14,"context_line":""},{"line_number":15,"context_line":"Closes-Bug: 1584076"},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"7aa08908_10395da4","line":13,"range":{"start_line":10,"start_character":0,"end_line":13,"end_character":22},"updated":"2016-06-14 19:47:22.000000000","message":"This needs to change because we are avoiding the ACL update altogether now if there is no change to the is_public attribute. Does that make sense?","commit_id":"08c468436887250af3cec174011be60cd192374e"}],"glance/api/v1/images.py":[{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"0bd9d05d1291ed703f35fcb885fa5b835b3e00d7","unresolved":false,"context_lines":[{"line_number":1017,"context_line":"        if orig_or_updated_loc:"},{"line_number":1018,"context_line":"            try:"},{"line_number":1019,"context_line":"                if is_public is None:"},{"line_number":1020,"context_line":"                \tis_public \u003d orig_image_meta[\u0027is_public\u0027]"},{"line_number":1021,"context_line":"                self.update_store_acls(req, id, orig_or_updated_loc,"},{"line_number":1022,"context_line":"                                       public\u003dis_public)"},{"line_number":1023,"context_line":"            except store.BadStoreUri:"}],"source_content_type":"text/x-python","patch_set":1,"id":"bab6814e_decaf46d","line":1020,"range":{"start_line":1020,"start_character":16,"end_line":1020,"end_character":17},"updated":"2016-05-24 17:37:15.000000000","message":"did you use a tab here instead of ws?","commit_id":"1ec4cd57acc4b4833bcd39037566041f405fd67e"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"4ffe6d207949f109eaa904b0d7abfcbe65f3d2d7","unresolved":false,"context_lines":[{"line_number":1016,"context_line":"        orig_or_updated_loc \u003d location or orig_image_meta.get(\u0027location\u0027)"},{"line_number":1017,"context_line":"        if orig_or_updated_loc:"},{"line_number":1018,"context_line":"            try:"},{"line_number":1019,"context_line":"                if is_public is None:"},{"line_number":1020,"context_line":"                        is_public \u003d orig_image_meta[\u0027is_public\u0027]"},{"line_number":1021,"context_line":"                self.update_store_acls(req, id, orig_or_updated_loc,"},{"line_number":1022,"context_line":"                                       public\u003dis_public)"},{"line_number":1023,"context_line":"            except store.BadStoreUri:"}],"source_content_type":"text/x-python","patch_set":2,"id":"bab6814e_5c63771a","line":1020,"range":{"start_line":1019,"start_character":15,"end_line":1020,"end_character":64},"updated":"2016-05-25 15:11:55.000000000","message":"Isn\u0027t this effectively a no-op in this case?\nIf I understand this correctly, we are trying to set acls even if the user didn\u0027t intend to change the public attribute of an image. When it\u0027s an updated location, may be it makes sense to do so. But, if it is same location, why update the acls again?\n\nWhat if I suggest something like this: if we have an updated location, set acls regardless of whether there is a change to public or not. But, if it is the same location and no change to public, don\u0027t set acls.\n\nDoes it make sense?","commit_id":"ce8a24a99acd4062399925faa07e451279d17a74"},{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"6d6c187c3c169de825087053b2ba325c9215cf61","unresolved":false,"context_lines":[{"line_number":1016,"context_line":"        orig_or_updated_loc \u003d location or orig_image_meta.get(\u0027location\u0027)"},{"line_number":1017,"context_line":"        if orig_or_updated_loc:"},{"line_number":1018,"context_line":"            try:"},{"line_number":1019,"context_line":"                if is_public is not None or location is not None:"},{"line_number":1020,"context_line":"                        self.update_store_acls(req, id, orig_or_updated_loc,"},{"line_number":1021,"context_line":"                                       public\u003dis_public)"},{"line_number":1022,"context_line":"            except store.BadStoreUri:"}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_a2bb2e0e","line":1019,"updated":"2016-05-26 11:54:58.000000000","message":"Looks like the root cause is different. The bug happens because if image is public but it was not specified in image_meta we are passing public \u003d None. So this updates acls incorrectly. I guess it can be fixed also if we will request is_public from database if it is not specified in image_meta.\nFor me that solution looks better because looks like the only possible way to update acls is update an image itself. So somebody may rely on that behavior.","commit_id":"276ee4224aa4559fbd7668fbc9253698589e7684"},{"author":{"_account_id":14676,"name":"Kairat Kushaev","email":"kkushaev@mirantis.com","username":"kairat_kushaev"},"change_message_id":"68ddf6b75f1d5ae41aaf008248a4db2de53912be","unresolved":false,"context_lines":[{"line_number":1016,"context_line":"        orig_or_updated_loc \u003d location or orig_image_meta.get(\u0027location\u0027)"},{"line_number":1017,"context_line":"        if orig_or_updated_loc:"},{"line_number":1018,"context_line":"            try:"},{"line_number":1019,"context_line":"                if is_public is not None or location is not None:"},{"line_number":1020,"context_line":"                        self.update_store_acls(req, id, orig_or_updated_loc,"},{"line_number":1021,"context_line":"                                       public\u003dis_public)"},{"line_number":1022,"context_line":"            except store.BadStoreUri:"}],"source_content_type":"text/x-python","patch_set":3,"id":"9abb7d3a_d8787cfb","line":1019,"in_reply_to":"bab6814e_1c7a5624","updated":"2016-05-27 09:25:25.000000000","message":"Ah, yeah, you are right. Thanks for clarification.","commit_id":"276ee4224aa4559fbd7668fbc9253698589e7684"},{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"23f33017eadc8e7d4e1e262c8cbc2bf0bd962709","unresolved":false,"context_lines":[{"line_number":1016,"context_line":"        orig_or_updated_loc \u003d location or orig_image_meta.get(\u0027location\u0027)"},{"line_number":1017,"context_line":"        if orig_or_updated_loc:"},{"line_number":1018,"context_line":"            try:"},{"line_number":1019,"context_line":"                if is_public is not None or location is not None:"},{"line_number":1020,"context_line":"                        self.update_store_acls(req, id, orig_or_updated_loc,"},{"line_number":1021,"context_line":"                                       public\u003dis_public)"},{"line_number":1022,"context_line":"            except store.BadStoreUri:"}],"source_content_type":"text/x-python","patch_set":3,"id":"bab6814e_1c7a5624","line":1019,"in_reply_to":"bab6814e_a2bb2e0e","updated":"2016-05-26 14:30:36.000000000","message":"Kairat, that is similar to what is attempted in the 2nd patchset. While what you say works, I think it is unnecessary to update ACLs when there is no need for it. If it is the same location and same public visibility, why update the acls?","commit_id":"276ee4224aa4559fbd7668fbc9253698589e7684"}]}