)]}'
{"glance/common/wsgi.py":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"2702e0dde32917620c3dd42a30ffc4e414a12b0e","unresolved":true,"context_lines":[{"line_number":513,"context_line":""},{"line_number":514,"context_line":"    def start_wsgi(self):"},{"line_number":515,"context_line":"        workers \u003d get_num_workers()"},{"line_number":516,"context_line":"        self.pool \u003d self.create_pool()"},{"line_number":517,"context_line":"        if workers \u003d\u003d 0:"},{"line_number":518,"context_line":"            # Useful for profiling, test, debug etc."},{"line_number":519,"context_line":"            self.pool.spawn_n(self._single_run, self.application, self.sock)"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f1e1799_0ec2894c","line":516,"updated":"2021-03-02 14:13:11.000000000","message":"Do we really need separate 1000 thread greenlet pool for this?","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"51a88b50b636e6dd465f7e1f7f31e47efbef5c0c","unresolved":true,"context_lines":[{"line_number":513,"context_line":""},{"line_number":514,"context_line":"    def start_wsgi(self):"},{"line_number":515,"context_line":"        workers \u003d get_num_workers()"},{"line_number":516,"context_line":"        self.pool \u003d self.create_pool()"},{"line_number":517,"context_line":"        if workers \u003d\u003d 0:"},{"line_number":518,"context_line":"            # Useful for profiling, test, debug etc."},{"line_number":519,"context_line":"            self.pool.spawn_n(self._single_run, self.application, self.sock)"}],"source_content_type":"text/x-python","patch_set":9,"id":"87a119ba_fe453ae7","line":516,"in_reply_to":"7f1e1799_0ec2894c","updated":"2021-03-02 19:49:39.000000000","message":"This is moved from L511 to outside the conditional so that it\u0027s always set regardless of the worker size config. Also, it\u0027s reusing the same pool as we use for spinning up API workers so that all the existing thread spawning and accounting works.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"8bcf7f15da6ac081f348d993a9edec6612387282","unresolved":true,"context_lines":[{"line_number":513,"context_line":""},{"line_number":514,"context_line":"    def start_wsgi(self):"},{"line_number":515,"context_line":"        workers \u003d get_num_workers()"},{"line_number":516,"context_line":"        self.pool \u003d self.create_pool()"},{"line_number":517,"context_line":"        if workers \u003d\u003d 0:"},{"line_number":518,"context_line":"            # Useful for profiling, test, debug etc."},{"line_number":519,"context_line":"            self.pool.spawn_n(self._single_run, self.application, self.sock)"}],"source_content_type":"text/x-python","patch_set":9,"id":"c885e25f_bae2be95","line":516,"in_reply_to":"87a119ba_fe453ae7","updated":"2021-03-03 14:17:26.000000000","message":"Yes, you moved it from when there is 0 workers defined to create thread pool for the service to run, to always create that pool. The pool is only reused if we run glance-api without workers, otherwise each worker will spin up a pool for themselves. Thus I was asking if we really want to have this pool initialized just for this purpose as 0 workers is very rare case outside of CI.\n\nHonestly I do not know which is better, to run in a dedicated proper thread or eventlet pool, there also should not be difference if it\u0027s 100 or 1000 greenthread pool as long as we don\u0027t spawn stuff in there. But this definitely does create an extra pool just for the single run cleanup in normal situations.\n\nJust wanted to make sure this was intentional.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f68c3317dde1f313c63336d3e7d124d18cdd7588","unresolved":true,"context_lines":[{"line_number":513,"context_line":""},{"line_number":514,"context_line":"    def start_wsgi(self):"},{"line_number":515,"context_line":"        workers \u003d get_num_workers()"},{"line_number":516,"context_line":"        self.pool \u003d self.create_pool()"},{"line_number":517,"context_line":"        if workers \u003d\u003d 0:"},{"line_number":518,"context_line":"            # Useful for profiling, test, debug etc."},{"line_number":519,"context_line":"            self.pool.spawn_n(self._single_run, self.application, self.sock)"}],"source_content_type":"text/x-python","patch_set":9,"id":"a1573796_59185ffd","line":516,"in_reply_to":"c885e25f_bae2be95","updated":"2021-03-03 14:52:37.000000000","message":"It was, and I opted for the common interface (which is what I meant by \"same pool\") instead of a random one-off thread spawn, which I think is fine. I\u0027m quite sure it doesn\u0027t pre-allocate threads or anything like that.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"}],"glance/common/wsgi_app.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"2eea7677fc184000bef04454e31f5ceb313485d1","unresolved":true,"context_lines":[{"line_number":100,"context_line":"    cleanup_thread \u003d threading.Thread("},{"line_number":101,"context_line":"        target\u003dcleaner.clean_orphaned_staging_residue,"},{"line_number":102,"context_line":"        daemon\u003dTrue)"},{"line_number":103,"context_line":"    cleanup_thread.start()"},{"line_number":104,"context_line":""},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"def init_app():"}],"source_content_type":"text/x-python","patch_set":11,"id":"c8e3bb19_1ec1d773","line":103,"range":{"start_line":103,"start_character":4,"end_line":103,"end_character":26},"updated":"2021-03-03 05:28:06.000000000","message":"Similar comment related to GlanceException here?","commit_id":"5b99ae1947655518edce2f8a059b54ee9d9ad433"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"267604e6ac1af3c9e436a01558fce49f76d10bca","unresolved":true,"context_lines":[{"line_number":135,"context_line":"        glance_store.create_stores(CONF)"},{"line_number":136,"context_line":"        glance_store.verify_default_store()"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    run_staging_cleanup()"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    _setup_os_profiler()"},{"line_number":141,"context_line":"    _validate_policy_enforcement_configuration()"}],"source_content_type":"text/x-python","patch_set":14,"id":"fd959053_33cf4251","line":138,"updated":"2021-03-04 12:57:40.000000000","message":"NIT: noticed this only now. Should we kick off this potentially long running thread only after the _validate_policy_enforcement_configuration() on L:141 has not killed the service right off.\n\nNot a biggie, should only matter around non-clean upgrades.","commit_id":"232177e68c547a815c31a2f30b6d1f95cdb3098d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"dbf9508048e6a1e970cb87dc7f1768d60b7db26a","unresolved":true,"context_lines":[{"line_number":135,"context_line":"        glance_store.create_stores(CONF)"},{"line_number":136,"context_line":"        glance_store.verify_default_store()"},{"line_number":137,"context_line":""},{"line_number":138,"context_line":"    run_staging_cleanup()"},{"line_number":139,"context_line":""},{"line_number":140,"context_line":"    _setup_os_profiler()"},{"line_number":141,"context_line":"    _validate_policy_enforcement_configuration()"}],"source_content_type":"text/x-python","patch_set":14,"id":"0cbaddf0_15ab2948","line":138,"in_reply_to":"fd959053_33cf4251","updated":"2021-03-04 14:21:29.000000000","message":"Sure, if we have to respin this I\u0027ll reorder those things.","commit_id":"232177e68c547a815c31a2f30b6d1f95cdb3098d"}],"glance/housekeeping.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"a2ff52cc49c819c8473387df474b6bf237bb7d9f","unresolved":true,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    @property"},{"line_number":35,"context_line":"    def staging_store_path(self):"},{"line_number":36,"context_line":"        separator, staging_dir \u003d store_utils.get_dir_separator()"},{"line_number":37,"context_line":"        expected_prefix \u003d \u0027file://\u0027"},{"line_number":38,"context_line":"        if not staging_dir.startswith(expected_prefix):"},{"line_number":39,"context_line":"            raise exception.GlanceException("}],"source_content_type":"text/x-python","patch_set":3,"id":"659eb2fd_9d05a4e6","line":36,"range":{"start_line":36,"start_character":8,"end_line":36,"end_character":64},"updated":"2021-02-23 11:57:40.000000000","message":"This cleanup will work only if multiple backends are enabled, because this method returns path according to reserved store os_glance_staging_store\n\nIn case of single store we use node_staging_uri for staging the data.","commit_id":"db2770439155572d29b65b1a3434539012dfb9a9"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"109c4180bf9fccfd83f321c96de158f6d418285d","unresolved":true,"context_lines":[{"line_number":33,"context_line":""},{"line_number":34,"context_line":"    @property"},{"line_number":35,"context_line":"    def staging_store_path(self):"},{"line_number":36,"context_line":"        separator, staging_dir \u003d store_utils.get_dir_separator()"},{"line_number":37,"context_line":"        expected_prefix \u003d \u0027file://\u0027"},{"line_number":38,"context_line":"        if not staging_dir.startswith(expected_prefix):"},{"line_number":39,"context_line":"            raise exception.GlanceException("}],"source_content_type":"text/x-python","patch_set":3,"id":"953183ef_bd01478d","line":36,"range":{"start_line":36,"start_character":8,"end_line":36,"end_character":64},"in_reply_to":"659eb2fd_9d05a4e6","updated":"2021-02-23 14:29:14.000000000","message":"I just realized that this morning when looking at a non-multistore job with my post-run test 😊\n\nWill update this, thanks.","commit_id":"db2770439155572d29b65b1a3434539012dfb9a9"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"2702e0dde32917620c3dd42a30ffc4e414a12b0e","unresolved":true,"context_lines":[{"line_number":41,"context_line":"            staging_dir \u003d CONF.node_staging_uri"},{"line_number":42,"context_line":"        expected_prefix \u003d \u0027file://\u0027"},{"line_number":43,"context_line":"        if not staging_dir.startswith(expected_prefix):"},{"line_number":44,"context_line":"            raise exception.GlanceException("},{"line_number":45,"context_line":"                \u0027Unexpected scheme in staging store; \u0027"},{"line_number":46,"context_line":"                \u0027unable to scan for residue\u0027)"},{"line_number":47,"context_line":"        return staging_dir[len(expected_prefix):]"}],"source_content_type":"text/x-python","patch_set":9,"id":"8080be7c_25d82536","line":44,"updated":"2021-03-02 14:13:11.000000000","message":"We should catch this somewhere.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"8bcf7f15da6ac081f348d993a9edec6612387282","unresolved":true,"context_lines":[{"line_number":41,"context_line":"            staging_dir \u003d CONF.node_staging_uri"},{"line_number":42,"context_line":"        expected_prefix \u003d \u0027file://\u0027"},{"line_number":43,"context_line":"        if not staging_dir.startswith(expected_prefix):"},{"line_number":44,"context_line":"            raise exception.GlanceException("},{"line_number":45,"context_line":"                \u0027Unexpected scheme in staging store; \u0027"},{"line_number":46,"context_line":"                \u0027unable to scan for residue\u0027)"},{"line_number":47,"context_line":"        return staging_dir[len(expected_prefix):]"}],"source_content_type":"text/x-python","patch_set":9,"id":"a1eb6e06_5961bf01","line":44,"in_reply_to":"76de89ce_03633e51","updated":"2021-03-03 14:17:26.000000000","message":"I think that\u0027s already enforced in the store side too. But sure, I was more wondering if we really want to trip on it here should it be possible to slip invalid value through.\n\nMy initial thought was more on the line of catch it, log it, no-op as can\u0027t use it.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"51a88b50b636e6dd465f7e1f7f31e47efbef5c0c","unresolved":true,"context_lines":[{"line_number":41,"context_line":"            staging_dir \u003d CONF.node_staging_uri"},{"line_number":42,"context_line":"        expected_prefix \u003d \u0027file://\u0027"},{"line_number":43,"context_line":"        if not staging_dir.startswith(expected_prefix):"},{"line_number":44,"context_line":"            raise exception.GlanceException("},{"line_number":45,"context_line":"                \u0027Unexpected scheme in staging store; \u0027"},{"line_number":46,"context_line":"                \u0027unable to scan for residue\u0027)"},{"line_number":47,"context_line":"        return staging_dir[len(expected_prefix):]"}],"source_content_type":"text/x-python","patch_set":9,"id":"76de89ce_03633e51","line":44,"in_reply_to":"8080be7c_25d82536","updated":"2021-03-02 19:49:39.000000000","message":"Note that this is just a config format check, and staging in general won\u0027t work if this is not set properly, thus there really can\u0027t be anything to clean up. It\u0027s also implicitly going to always be there if we\u0027re in multistore mode, because of get_dir_separator().\n\nBut sure, I\u0027ll dereference this somewhere in the main thread and force an exit if we hit it before we spawn the cleaner. Since the case below (where import isn\u0027t used at all) should still pass the check with the default \"file:///tmp/staging\" config, we can make this an early canary for an invalid staging scheme.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"2702e0dde32917620c3dd42a30ffc4e414a12b0e","unresolved":true,"context_lines":[{"line_number":54,"context_line":"            image \u003d self.db.image_get(self.context, image_id)"},{"line_number":55,"context_line":"            # FIXME(danms): Maybe check that it\u0027s not deleted or"},{"line_number":56,"context_line":"            # something else like state, size, etc"},{"line_number":57,"context_line":"            return not image[\u0027deleted\u0027]"},{"line_number":58,"context_line":"        except exception.ImageNotFound:"},{"line_number":59,"context_line":"            return False"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5b5e3706_c4de378e","line":57,"range":{"start_line":57,"start_character":29,"end_line":57,"end_character":38},"updated":"2021-03-02 14:13:11.000000000","message":"This should cover: \u0027deleted\u0027, \u0027pending_delete\u0027, \u0027killed\u0027 and probably \u0027deactivated\u0027 too. The last being up for debate.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"51a88b50b636e6dd465f7e1f7f31e47efbef5c0c","unresolved":true,"context_lines":[{"line_number":54,"context_line":"            image \u003d self.db.image_get(self.context, image_id)"},{"line_number":55,"context_line":"            # FIXME(danms): Maybe check that it\u0027s not deleted or"},{"line_number":56,"context_line":"            # something else like state, size, etc"},{"line_number":57,"context_line":"            return not image[\u0027deleted\u0027]"},{"line_number":58,"context_line":"        except exception.ImageNotFound:"},{"line_number":59,"context_line":"            return False"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"671cdab0_76d01f95","line":57,"range":{"start_line":57,"start_character":29,"end_line":57,"end_character":38},"in_reply_to":"5b5e3706_c4de378e","updated":"2021-03-02 19:49:39.000000000","message":"Why? Checking deleted means we only run this cleanup once the record has been deleted (as in Image.deleted!\u003d0 in the database), which should happen after all of these other cases, right?","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"8bcf7f15da6ac081f348d993a9edec6612387282","unresolved":true,"context_lines":[{"line_number":54,"context_line":"            image \u003d self.db.image_get(self.context, image_id)"},{"line_number":55,"context_line":"            # FIXME(danms): Maybe check that it\u0027s not deleted or"},{"line_number":56,"context_line":"            # something else like state, size, etc"},{"line_number":57,"context_line":"            return not image[\u0027deleted\u0027]"},{"line_number":58,"context_line":"        except exception.ImageNotFound:"},{"line_number":59,"context_line":"            return False"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"d06b429a_799b5db9","line":57,"range":{"start_line":57,"start_character":29,"end_line":57,"end_character":38},"in_reply_to":"671cdab0_76d01f95","updated":"2021-03-03 14:17:26.000000000","message":"Hmm-m, I mixed that with status, sorry. I think we even flip that switch when image goes to \"pending_delete\" status so it likely indeed covers at least the most important cases.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f68c3317dde1f313c63336d3e7d124d18cdd7588","unresolved":false,"context_lines":[{"line_number":54,"context_line":"            image \u003d self.db.image_get(self.context, image_id)"},{"line_number":55,"context_line":"            # FIXME(danms): Maybe check that it\u0027s not deleted or"},{"line_number":56,"context_line":"            # something else like state, size, etc"},{"line_number":57,"context_line":"            return not image[\u0027deleted\u0027]"},{"line_number":58,"context_line":"        except exception.ImageNotFound:"},{"line_number":59,"context_line":"            return False"},{"line_number":60,"context_line":""}],"source_content_type":"text/x-python","patch_set":9,"id":"5efdff4a_62818f1c","line":57,"range":{"start_line":57,"start_character":29,"end_line":57,"end_character":38},"in_reply_to":"d06b429a_799b5db9","updated":"2021-03-03 14:52:37.000000000","message":"Ack","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"2702e0dde32917620c3dd42a30ffc4e414a12b0e","unresolved":true,"context_lines":[{"line_number":75,"context_line":"    def clean_orphaned_staging_residue(self):"},{"line_number":76,"context_line":"        try:"},{"line_number":77,"context_line":"            files \u003d os.listdir(self.staging_store_path)"},{"line_number":78,"context_line":"        except FileNotFoundError:"},{"line_number":79,"context_line":"            # NOTE(danms): If we cannot list the staging dir, there is"},{"line_number":80,"context_line":"            # clearly nothing left from a previous run, so nothing to"},{"line_number":81,"context_line":"            # clean up."}],"source_content_type":"text/x-python","patch_set":9,"id":"b16fe990_80c71e13","line":78,"updated":"2021-03-02 14:13:11.000000000","message":"The only reason we would be getting this is because the staging directory does not exist (as it\u0027s looking to be set right per us getting something back from staging_store_path()). Empty folder would just return empty list. Definitely not an error we should just ignore (is the host missing a mount when the service came up for example). We should at least LOG.warning() here.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"8bcf7f15da6ac081f348d993a9edec6612387282","unresolved":true,"context_lines":[{"line_number":75,"context_line":"    def clean_orphaned_staging_residue(self):"},{"line_number":76,"context_line":"        try:"},{"line_number":77,"context_line":"            files \u003d os.listdir(self.staging_store_path)"},{"line_number":78,"context_line":"        except FileNotFoundError:"},{"line_number":79,"context_line":"            # NOTE(danms): If we cannot list the staging dir, there is"},{"line_number":80,"context_line":"            # clearly nothing left from a previous run, so nothing to"},{"line_number":81,"context_line":"            # clean up."}],"source_content_type":"text/x-python","patch_set":9,"id":"7704f32c_16c658e9","line":78,"in_reply_to":"1af302e7_c1d1463b","updated":"2021-03-03 14:17:26.000000000","message":"Like said, just log it instead of pretending that everything is fine and do nothing. I didn\u0027t say we should trip the service start over it, just indicate that we have found potential problem.\n\nBTW due to the interop and defcore requirements at the time, your scenario 1 is not supported configuration so we really don\u0027t need to worry too much about it. There was clear requirement for at least one import option being avaiable, which in return puts clear requirement for staging being configured.\n\nFor 2. might happen once, yeii, it\u0027s not like we don\u0027t log anything less valueable across the services with constant spam. So having the service logging \"Hey, couldn\u0027t access staging!\" once during the lifecycle of the deployment for something that can be passed by \"Yeah, we\u0027re getting to it.\" is really not that horrible thing, when if it gets logged again it might be actual issue that has gone otherwise unnoticed.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"51a88b50b636e6dd465f7e1f7f31e47efbef5c0c","unresolved":true,"context_lines":[{"line_number":75,"context_line":"    def clean_orphaned_staging_residue(self):"},{"line_number":76,"context_line":"        try:"},{"line_number":77,"context_line":"            files \u003d os.listdir(self.staging_store_path)"},{"line_number":78,"context_line":"        except FileNotFoundError:"},{"line_number":79,"context_line":"            # NOTE(danms): If we cannot list the staging dir, there is"},{"line_number":80,"context_line":"            # clearly nothing left from a previous run, so nothing to"},{"line_number":81,"context_line":"            # clean up."}],"source_content_type":"text/x-python","patch_set":9,"id":"1af302e7_c1d1463b","line":78,"in_reply_to":"b16fe990_80c71e13","updated":"2021-03-02 19:49:39.000000000","message":"IMHO, it isn\u0027t our job to enforce that staging is present and working, just to not die if it\u0027s not there. The staging directory doesn\u0027t need to be present until/unless an actual image stage (or import that uses it) is initiated. So, the following completely legit scenarios would result in us finding a missing staging directory, where logging a warning would be invalid:\n\n 1. I disable all import methods and have no staging directory at all\n 2. My deployment tool is parallelizing the initial setup and we start the glance-api worker before we have created the staging directory (day 0), all of which happens before we even have a service catalog to find this endpoint.\n\nJust like the must-probe-cinder-at-start-or-fail discussion we had, I don\u0027t think this should be overly concerned about a missing staging directory at startup. Its only purpose is to clean residue, if present, and if it\u0027s not there, there\u0027s clearly no residue. When we actually need a staging directory, that\u0027s where we should log a warning (really an error at that point) about it being missing.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"2702e0dde32917620c3dd42a30ffc4e414a12b0e","unresolved":true,"context_lines":[{"line_number":87,"context_line":"                  len(files))"},{"line_number":88,"context_line":"        cleaned \u003d ignored \u003d error \u003d 0"},{"line_number":89,"context_line":"        for filename in files:"},{"line_number":90,"context_line":"            if not self.is_image_id(filename):"},{"line_number":91,"context_line":"                # NOTE(danms): We should probably either have a config option"},{"line_number":92,"context_line":"                # that decides what to do here (i.e. reap or ignore), or decide"},{"line_number":93,"context_line":"                # that this is not okay and just nuke anything we find."}],"source_content_type":"text/x-python","patch_set":9,"id":"af45e2a2_c864a076","line":90,"updated":"2021-03-02 14:13:11.000000000","message":"This will catch all staged data from conversion and decompress plugins. Conversion writes to \u003cimage-id\u003e.\u003ctarget\u003e target being the the string \"name\" of target format and decompression plugin writes to \u003cimage_id\u003e\".uc\"","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"8bcf7f15da6ac081f348d993a9edec6612387282","unresolved":true,"context_lines":[{"line_number":87,"context_line":"                  len(files))"},{"line_number":88,"context_line":"        cleaned \u003d ignored \u003d error \u003d 0"},{"line_number":89,"context_line":"        for filename in files:"},{"line_number":90,"context_line":"            if not self.is_image_id(filename):"},{"line_number":91,"context_line":"                # NOTE(danms): We should probably either have a config option"},{"line_number":92,"context_line":"                # that decides what to do here (i.e. reap or ignore), or decide"},{"line_number":93,"context_line":"                # that this is not okay and just nuke anything we find."}],"source_content_type":"text/x-python","patch_set":9,"id":"d02c4974_d571d0ef","line":90,"in_reply_to":"7d58a9b7_1799963b","updated":"2021-03-03 14:17:26.000000000","message":"This condition catches those cases and leaves the data behind as ignored.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"51a88b50b636e6dd465f7e1f7f31e47efbef5c0c","unresolved":true,"context_lines":[{"line_number":87,"context_line":"                  len(files))"},{"line_number":88,"context_line":"        cleaned \u003d ignored \u003d error \u003d 0"},{"line_number":89,"context_line":"        for filename in files:"},{"line_number":90,"context_line":"            if not self.is_image_id(filename):"},{"line_number":91,"context_line":"                # NOTE(danms): We should probably either have a config option"},{"line_number":92,"context_line":"                # that decides what to do here (i.e. reap or ignore), or decide"},{"line_number":93,"context_line":"                # that this is not okay and just nuke anything we find."}],"source_content_type":"text/x-python","patch_set":9,"id":"7d58a9b7_1799963b","line":90,"in_reply_to":"af45e2a2_c864a076","updated":"2021-03-02 19:49:39.000000000","message":"Those suffixes will ensure that this does *not* catch those things, as it will only pass if the filename is exactly an image uuid.\n\nOr are you saying you\u0027d like to not see the LOG.info() below for those items?","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"f68c3317dde1f313c63336d3e7d124d18cdd7588","unresolved":true,"context_lines":[{"line_number":87,"context_line":"                  len(files))"},{"line_number":88,"context_line":"        cleaned \u003d ignored \u003d error \u003d 0"},{"line_number":89,"context_line":"        for filename in files:"},{"line_number":90,"context_line":"            if not self.is_image_id(filename):"},{"line_number":91,"context_line":"                # NOTE(danms): We should probably either have a config option"},{"line_number":92,"context_line":"                # that decides what to do here (i.e. reap or ignore), or decide"},{"line_number":93,"context_line":"                # that this is not okay and just nuke anything we find."}],"source_content_type":"text/x-python","patch_set":9,"id":"1d9a798c_faa64b1f","line":90,"in_reply_to":"d02c4974_d571d0ef","updated":"2021-03-03 14:52:37.000000000","message":"Okay, so you\u0027re asking to have those be cleaned up, gotcha.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"2702e0dde32917620c3dd42a30ffc4e414a12b0e","unresolved":true,"context_lines":[{"line_number":91,"context_line":"                # NOTE(danms): We should probably either have a config option"},{"line_number":92,"context_line":"                # that decides what to do here (i.e. reap or ignore), or decide"},{"line_number":93,"context_line":"                # that this is not okay and just nuke anything we find."},{"line_number":94,"context_line":"                LOG.info(_LI(\u0027Staging directory contains unexpected non-image \u0027"},{"line_number":95,"context_line":"                             \u0027file %r; ignoring\u0027),"},{"line_number":96,"context_line":"                         filename)"},{"line_number":97,"context_line":"                ignored +\u003d 1"}],"source_content_type":"text/x-python","patch_set":9,"id":"b8bcb528_36ebbb31","line":94,"updated":"2021-03-02 14:13:11.000000000","message":"We should not log fault conditions as Info https://docs.python.org/3/howto/logging.html#when-to-use-logging.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"51a88b50b636e6dd465f7e1f7f31e47efbef5c0c","unresolved":true,"context_lines":[{"line_number":91,"context_line":"                # NOTE(danms): We should probably either have a config option"},{"line_number":92,"context_line":"                # that decides what to do here (i.e. reap or ignore), or decide"},{"line_number":93,"context_line":"                # that this is not okay and just nuke anything we find."},{"line_number":94,"context_line":"                LOG.info(_LI(\u0027Staging directory contains unexpected non-image \u0027"},{"line_number":95,"context_line":"                             \u0027file %r; ignoring\u0027),"},{"line_number":96,"context_line":"                         filename)"},{"line_number":97,"context_line":"                ignored +\u003d 1"}],"source_content_type":"text/x-python","patch_set":9,"id":"6dd5f8a6_272b9571","line":94,"in_reply_to":"b8bcb528_36ebbb31","updated":"2021-03-02 19:49:39.000000000","message":"Is it a fault condition? If it is, then we should just nuke the files we find here (see note above). If someone mounts a dedicated filesystem here, we\u0027ll see things like \"lost+found\" and potentially other things like \".zfs\" in that case. I definitely don\u0027t think we should always LOG.warn() on situations like this, and maybe LOG.info() is even too much.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"2702e0dde32917620c3dd42a30ffc4e414a12b0e","unresolved":true,"context_lines":[{"line_number":102,"context_line":"                ignored +\u003d 1"},{"line_number":103,"context_line":"                continue"},{"line_number":104,"context_line":"            path \u003d os.path.join(self.staging_store_path, filename)"},{"line_number":105,"context_line":"            LOG.warning(_LW(\u0027Stale staging residue found for image \u0027"},{"line_number":106,"context_line":"                            \u0027%(uuid)s: %(file)r; deleting now.\u0027),"},{"line_number":107,"context_line":"                        {\u0027uuid\u0027: filename, \u0027file\u0027: path})"},{"line_number":108,"context_line":"            if self.delete_file(path):"}],"source_content_type":"text/x-python","patch_set":9,"id":"7f9335f2_3c1ad59b","line":105,"updated":"2021-03-02 14:13:11.000000000","message":"I think this is our normal expected operation so Info or debug would be correct level here.","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"51a88b50b636e6dd465f7e1f7f31e47efbef5c0c","unresolved":false,"context_lines":[{"line_number":102,"context_line":"                ignored +\u003d 1"},{"line_number":103,"context_line":"                continue"},{"line_number":104,"context_line":"            path \u003d os.path.join(self.staging_store_path, filename)"},{"line_number":105,"context_line":"            LOG.warning(_LW(\u0027Stale staging residue found for image \u0027"},{"line_number":106,"context_line":"                            \u0027%(uuid)s: %(file)r; deleting now.\u0027),"},{"line_number":107,"context_line":"                        {\u0027uuid\u0027: filename, \u0027file\u0027: path})"},{"line_number":108,"context_line":"            if self.delete_file(path):"}],"source_content_type":"text/x-python","patch_set":9,"id":"e593516b_6c6d1fc4","line":105,"in_reply_to":"7f9335f2_3c1ad59b","updated":"2021-03-02 19:49:39.000000000","message":"Done","commit_id":"928597a3ae8caa20f8fd56fb54f9cde800d61496"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"9e644fdb2f139dc02b103344446a8c4906d40dab","unresolved":true,"context_lines":[{"line_number":50,"context_line":"                \u0027unable to scan for residue\u0027)"},{"line_number":51,"context_line":"        return staging_dir[len(expected_prefix):]"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def is_image_id(self, filename):"},{"line_number":54,"context_line":"        return uuidutils.is_uuid_like(filename)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def is_valid_image(self, image_id):"}],"source_content_type":"text/x-python","patch_set":10,"id":"e7b6b754_4d63fa06","line":53,"range":{"start_line":53,"start_character":8,"end_line":53,"end_character":19},"updated":"2021-03-03 01:13:05.000000000","message":"Shouldn\u0027t this be a staticmethod?","commit_id":"fb2f59c180c61bf4dd0eb745755a6234cafd0d72"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"3fe473da80b0cba5033e8860cb5ea1c1b659d423","unresolved":true,"context_lines":[{"line_number":50,"context_line":"                \u0027unable to scan for residue\u0027)"},{"line_number":51,"context_line":"        return staging_dir[len(expected_prefix):]"},{"line_number":52,"context_line":""},{"line_number":53,"context_line":"    def is_image_id(self, filename):"},{"line_number":54,"context_line":"        return uuidutils.is_uuid_like(filename)"},{"line_number":55,"context_line":""},{"line_number":56,"context_line":"    def is_valid_image(self, image_id):"}],"source_content_type":"text/x-python","patch_set":10,"id":"f4731041_1259fec4","line":53,"range":{"start_line":53,"start_character":8,"end_line":53,"end_character":19},"in_reply_to":"e7b6b754_4d63fa06","updated":"2021-03-03 01:30:59.000000000","message":"Sure :)","commit_id":"fb2f59c180c61bf4dd0eb745755a6234cafd0d72"},{"author":{"_account_id":8122,"name":"Cyril Roelandt","email":"cyril@redhat.com","username":"cyril.roelandt.enovance"},"change_message_id":"9e644fdb2f139dc02b103344446a8c4906d40dab","unresolved":true,"context_lines":[{"line_number":62,"context_line":"        except exception.ImageNotFound:"},{"line_number":63,"context_line":"            return False"},{"line_number":64,"context_line":""},{"line_number":65,"context_line":"    def delete_file(self, path):"},{"line_number":66,"context_line":"        try:"},{"line_number":67,"context_line":"            os.remove(path)"},{"line_number":68,"context_line":"        except FileNotFoundError:"}],"source_content_type":"text/x-python","patch_set":10,"id":"7af848d0_f2e3efa7","line":65,"range":{"start_line":65,"start_character":8,"end_line":65,"end_character":19},"updated":"2021-03-03 01:13:05.000000000","message":"Ditto","commit_id":"fb2f59c180c61bf4dd0eb745755a6234cafd0d72"}],"glance/tests/functional/test_wsgi.py":[{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"9c3faa2501b9a76c13ec73f585c81700c260fe58","unresolved":true,"context_lines":[{"line_number":109,"context_line":"        # Create two more files in staging, one unrecognized one, and one uuid"},{"line_number":110,"context_line":"        # that matches nothing in the database."},{"line_number":111,"context_line":"        open(os.path.join(staging, \u0027foo\u0027), \u0027w\u0027)"},{"line_number":112,"context_line":"        open(os.path.join(staging, uuids.stale), \u0027w\u0027)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        # Restart the server. We set \"needs_database\" to False here to avoid"},{"line_number":115,"context_line":"        # recreating the database during startup, thus causing the server to"}],"source_content_type":"text/x-python","patch_set":12,"id":"0ff4c4dd_6640055a","line":112,"range":{"start_line":112,"start_character":8,"end_line":112,"end_character":53},"updated":"2021-03-03 17:37:07.000000000","message":"As we are now clearing stale temporary files created by plugins, can we add those and verify that those were deleted as well?\nFor example image conversion can create \u003cimage_id\u003e.raw temp file and image decompression can create \u003cimage_id\u003e.uc file","commit_id":"3934ab9dcc3e0049894e1086121558aa22384f9e"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"d24bb787a7ad9d322e785aedeb47de7557341faf","unresolved":true,"context_lines":[{"line_number":109,"context_line":"        # Create two more files in staging, one unrecognized one, and one uuid"},{"line_number":110,"context_line":"        # that matches nothing in the database."},{"line_number":111,"context_line":"        open(os.path.join(staging, \u0027foo\u0027), \u0027w\u0027)"},{"line_number":112,"context_line":"        open(os.path.join(staging, uuids.stale), \u0027w\u0027)"},{"line_number":113,"context_line":""},{"line_number":114,"context_line":"        # Restart the server. We set \"needs_database\" to False here to avoid"},{"line_number":115,"context_line":"        # recreating the database during startup, thus causing the server to"}],"source_content_type":"text/x-python","patch_set":12,"id":"eb4771b8_a55638d2","line":112,"range":{"start_line":112,"start_character":8,"end_line":112,"end_character":53},"in_reply_to":"0ff4c4dd_6640055a","updated":"2021-03-03 17:43:40.000000000","message":"I cover this in the housekeeping (unit) tests, but sure, I can add some more in here.","commit_id":"3934ab9dcc3e0049894e1086121558aa22384f9e"},{"author":{"_account_id":11904,"name":"Sean McGinnis","email":"sean.mcginnis@gmail.com","username":"SeanM"},"change_message_id":"f8e2381c41a5c5e94dd64c2be4b33c7579415d59","unresolved":true,"context_lines":[{"line_number":16,"context_line":"\"\"\"Tests for `glance.wsgi`.\"\"\""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import os"},{"line_number":19,"context_line":"from six.moves import http_client as http"},{"line_number":20,"context_line":"import socket"},{"line_number":21,"context_line":"import time"},{"line_number":22,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"1cc405c4_007a08ca","line":19,"range":{"start_line":19,"start_character":5,"end_line":19,"end_character":8},"updated":"2021-03-08 20:21:20.000000000","message":"six is a third party lib, so really should go with the oslo and requests imports below.\n\nBut bigger question is why introduce six at this point. Shouldn\u0027t need to do any py2 compat handling here.","commit_id":"232177e68c547a815c31a2f30b6d1f95cdb3098d"},{"author":{"_account_id":4393,"name":"Dan Smith","email":"dms@danplanet.com","username":"danms"},"change_message_id":"898879ac4aef578c104274c9f89d8ad41e6c6d1c","unresolved":true,"context_lines":[{"line_number":16,"context_line":"\"\"\"Tests for `glance.wsgi`.\"\"\""},{"line_number":17,"context_line":""},{"line_number":18,"context_line":"import os"},{"line_number":19,"context_line":"from six.moves import http_client as http"},{"line_number":20,"context_line":"import socket"},{"line_number":21,"context_line":"import time"},{"line_number":22,"context_line":""}],"source_content_type":"text/x-python","patch_set":14,"id":"92767167_14913a47","line":19,"range":{"start_line":19,"start_character":5,"end_line":19,"end_character":8},"in_reply_to":"1cc405c4_007a08ca","updated":"2021-03-08 21:21:32.000000000","message":"I was copying this from other places in the code, and a couple pokes at what the new name was didn\u0027t turn up anything. I haven\u0027t thought about it since I originally posted it. I\u0027m happy to fix this in a follow-up, or here if I have to respin.","commit_id":"232177e68c547a815c31a2f30b6d1f95cdb3098d"}]}