)]}'
{"glance/image_cache/drivers/sqlite.py":[{"author":{"_account_id":8158,"name":"Hemanth Makkapati","email":"makstacker+osgerrit@gmail.com","username":"hemanth.makkapati"},"change_message_id":"a132579e3994b900a889536d50bc70577406b893","unresolved":false,"context_lines":[{"line_number":499,"context_line":""},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"def delete_cached_file(path):"},{"line_number":502,"context_line":"    if os.path.exists(path):"},{"line_number":503,"context_line":"        LOG.debug(\"Deleting image cache file \u0027%s\u0027\", path)"},{"line_number":504,"context_line":"        try:"},{"line_number":505,"context_line":"            os.unlink(path)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_f7a7ae08","line":502,"range":{"start_line":502,"start_character":4,"end_line":502,"end_character":27},"updated":"2016-07-30 18:49:17.000000000","message":"In your commit message you say that \"admin removes one of the file before the cron job\". If the file is deleted before the cron job starts, then this if condition should take care of it. Unless someone times the manual delete so precisely that it happens after the exists check and before the unlink, it don\u0027t see the issue here. \n\nAm I missing anything here?","commit_id":"873f747c477e232514d62d9e0c0a510eeb284563"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"2ace703515c76f21f7ff05e41e2b8a511a8b84d2","unresolved":false,"context_lines":[{"line_number":499,"context_line":""},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"def delete_cached_file(path):"},{"line_number":502,"context_line":"    if os.path.exists(path):"},{"line_number":503,"context_line":"        LOG.debug(\"Deleting image cache file \u0027%s\u0027\", path)"},{"line_number":504,"context_line":"        try:"},{"line_number":505,"context_line":"            os.unlink(path)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_b6634d41","line":502,"range":{"start_line":502,"start_character":4,"end_line":502,"end_character":27},"in_reply_to":"bacf61ea_00ae53e5","updated":"2016-08-01 14:49:04.000000000","message":"Hemant, Brian:\n\nThis may never happen, or may happen if both corn-job(or any manual deleting) and os.unlink() are running in parallel(deleting sequence of images). Yes, again this is such a corner case.","commit_id":"873f747c477e232514d62d9e0c0a510eeb284563"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"be08fd1ddc68351e0fd940080125b69fcd2b9979","unresolved":false,"context_lines":[{"line_number":499,"context_line":""},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"def delete_cached_file(path):"},{"line_number":502,"context_line":"    if os.path.exists(path):"},{"line_number":503,"context_line":"        LOG.debug(\"Deleting image cache file \u0027%s\u0027\", path)"},{"line_number":504,"context_line":"        try:"},{"line_number":505,"context_line":"            os.unlink(path)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_00ae53e5","line":502,"range":{"start_line":502,"start_character":4,"end_line":502,"end_character":27},"in_reply_to":"bacf61ea_634140a9","updated":"2016-08-01 14:08:44.000000000","message":"I think the question Hemanth is asking is:\n\nUnder what circumstances will os.path.exists(path) be True while os.unlink(path) will raise an exception, given that the call to os.unlink(path) is almost immediately following the call to os.unlink(path)?  (If the admin has deleted an image manually, won\u0027t os.path.exists(path) be False, and the code will never see the os.unlink(path), and hence no exception will be raised?)\n\nI\u0027m curious about that, too.  (I\u0027m not being a wise-ass, I really want to understand the scenario you are fixing.)\n\nThat being said, I suppose that os.unlink() could fail for other reasons, so it\u0027s worth catching the exception here, I think, just on general principles.  But I\u0027d like to understand more about what\u0027s motivating this change you are proposing.","commit_id":"873f747c477e232514d62d9e0c0a510eeb284563"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"8df3e4feee5333cd3b235b8fb3df73c83e411196","unresolved":false,"context_lines":[{"line_number":499,"context_line":""},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"def delete_cached_file(path):"},{"line_number":502,"context_line":"    if os.path.exists(path):"},{"line_number":503,"context_line":"        LOG.debug(\"Deleting image cache file \u0027%s\u0027\", path)"},{"line_number":504,"context_line":"        try:"},{"line_number":505,"context_line":"            os.unlink(path)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_167699d6","line":502,"range":{"start_line":502,"start_character":4,"end_line":502,"end_character":27},"in_reply_to":"bacf61ea_b6634d41","updated":"2016-08-01 14:52:14.000000000","message":"Yes, I think we should keep the \"if os.path.exists(path)\".","commit_id":"873f747c477e232514d62d9e0c0a510eeb284563"},{"author":{"_account_id":20764,"name":"Sachin","email":"psachin@redhat.com","username":"psachin"},"change_message_id":"9df27acc46523afde3a07ee70f790670396b8960","unresolved":false,"context_lines":[{"line_number":499,"context_line":""},{"line_number":500,"context_line":""},{"line_number":501,"context_line":"def delete_cached_file(path):"},{"line_number":502,"context_line":"    if os.path.exists(path):"},{"line_number":503,"context_line":"        LOG.debug(\"Deleting image cache file \u0027%s\u0027\", path)"},{"line_number":504,"context_line":"        try:"},{"line_number":505,"context_line":"            os.unlink(path)"}],"source_content_type":"text/x-python","patch_set":1,"id":"bacf61ea_634140a9","line":502,"range":{"start_line":502,"start_character":4,"end_line":502,"end_character":27},"in_reply_to":"bacf61ea_f7a7ae08","updated":"2016-07-31 06:24:02.000000000","message":"Let me put it this way - I(say the admin) intentionally deleted the some arbitrary cached file. Now, if xattr.Driver.delete_all_cached_images() starts, the os.unlink() should certainly throw exception for that file. But that should not stop subsequent deletions.","commit_id":"873f747c477e232514d62d9e0c0a510eeb284563"}]}