)]}'
{"glance/async/flows/api_image_import.py":[{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"d1a49d913f394e4c2d66381b6ba686fb592133e2","unresolved":false,"context_lines":[{"line_number":38,"context_line":"# TODO(jokke): We should refactor the task implementations so that we do not"},{"line_number":39,"context_line":"# need to duplicate what we have already for example in base_import.py."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"class _DeleteFromFS(task.Task):"},{"line_number":42,"context_line":""},{"line_number":43,"context_line":"    def __init__(self, task_id, task_type):"},{"line_number":44,"context_line":"        self.task_id \u003d task_id"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a1ced50_e403cfb6","line":41,"range":{"start_line":41,"start_character":6,"end_line":41,"end_character":19},"updated":"2017-03-17 18:58:56.000000000","message":"s/_DeleteFromFS/_DeleteFromStore ?","commit_id":"530336b1983a2a828c77a03fdc1d730a87bd7ade"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"d1a49d913f394e4c2d66381b6ba686fb592133e2","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # NOTE(jokke): If we want to use other than \u0027file\u0027 store in the"},{"line_number":73,"context_line":"        # future, this is one thing that needs to change."},{"line_number":74,"context_line":"        try:"},{"line_number":75,"context_line":"            uri.index(\u0027file:///\u0027, 0)"},{"line_number":76,"context_line":"        except ValueError:"},{"line_number":77,"context_line":"            msg \u003d (_(\"%(task_id)s of %(task_type)s not configured \""},{"line_number":78,"context_line":"                     \"properly. Value of node_staging_uri must be \""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a1ced50_dae7883e","line":75,"range":{"start_line":75,"start_character":12,"end_line":75,"end_character":36},"updated":"2017-03-17 18:58:56.000000000","message":"While this will validate the format of the node_staging_uri, I am wondering what about the validity of the dir path provided?\nMaybe we should validate the path here before proceeding to building our store..\n\nOne way i can think of is catching an OSError via:\n    tempfile.TemporaryFile(dir\u003dself.uri[7:])","commit_id":"530336b1983a2a828c77a03fdc1d730a87bd7ade"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"d1a49d913f394e4c2d66381b6ba686fb592133e2","unresolved":false,"context_lines":[{"line_number":120,"context_line":""},{"line_number":121,"context_line":"    def execute(self):"},{"line_number":122,"context_line":"        \"\"\"Test the backend store and return the \u0027file_path\u0027\"\"\""},{"line_number":123,"context_line":"        return self.uri"},{"line_number":124,"context_line":""},{"line_number":125,"context_line":""},{"line_number":126,"context_line":"class _ImportToStore(task.Task):"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a1ced50_7ab934e9","line":123,"range":{"start_line":123,"start_character":8,"end_line":123,"end_character":23},"updated":"2017-03-17 18:58:56.000000000","message":"Do we intend to return the complete uri? Or just the file_path?","commit_id":"530336b1983a2a828c77a03fdc1d730a87bd7ade"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"d1a49d913f394e4c2d66381b6ba686fb592133e2","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        # Lets get to it and identify the different scenarios in the"},{"line_number":194,"context_line":"        # implementation"},{"line_number":195,"context_line":"        image \u003d self.image_repo.get(image_id)"},{"line_number":196,"context_line":"        image.status \u003d \u0027saving\u0027"},{"line_number":197,"context_line":"        self.image_repo.save(image)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        # NOTE(flaper87): Let\u0027s dance... and fall"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a1ced50_5ae8f87c","line":196,"range":{"start_line":196,"start_character":23,"end_line":196,"end_character":31},"updated":"2017-03-17 18:58:56.000000000","message":"A call to stage an image as per image imports workflow must be setting the state to \u0027uploading\u0027 as against \u0027saving\u0027, in order to differentiate image imports from direct uploads as per the spec [1].\n\n[1] http://specs.openstack.org/openstack/glance-specs/specs/mitaka/approved/image-import/image-import-refactor.html#image-detail\n\nDo we decide to take a different path and use \"saving\"? In that case we need to edit the spec.","commit_id":"530336b1983a2a828c77a03fdc1d730a87bd7ade"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"7e053749397f6490a608f70606b9c960f9e0e174","unresolved":false,"context_lines":[{"line_number":193,"context_line":"        # Lets get to it and identify the different scenarios in the"},{"line_number":194,"context_line":"        # implementation"},{"line_number":195,"context_line":"        image \u003d self.image_repo.get(image_id)"},{"line_number":196,"context_line":"        image.status \u003d \u0027saving\u0027"},{"line_number":197,"context_line":"        self.image_repo.save(image)"},{"line_number":198,"context_line":""},{"line_number":199,"context_line":"        # NOTE(flaper87): Let\u0027s dance... and fall"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a1ced50_39010e39","line":196,"range":{"start_line":196,"start_character":23,"end_line":196,"end_character":31},"in_reply_to":"1a1ced50_5ae8f87c","updated":"2017-03-22 00:11:17.000000000","message":"why not just use new_image.image_update with the correct from_state to set the status so we don\u0027t have a race condition?","commit_id":"530336b1983a2a828c77a03fdc1d730a87bd7ade"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"d1a49d913f394e4c2d66381b6ba686fb592133e2","unresolved":false,"context_lines":[{"line_number":241,"context_line":"        # NOTE(flaper87): We need to save the image again after the locations"},{"line_number":242,"context_line":"        # have been set in the image."},{"line_number":243,"context_line":"        self.image_repo.save(image)"},{"line_number":244,"context_line":""},{"line_number":245,"context_line":""},{"line_number":246,"context_line":"class _SaveImage(task.Task):"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a1ced50_baad6c33","line":244,"updated":"2017-03-17 18:58:56.000000000","message":"As per spec, we need to change the image status at this point to: \n\n  image.status \u003d \u0027importing\u0027","commit_id":"530336b1983a2a828c77a03fdc1d730a87bd7ade"},{"author":{"_account_id":21722,"name":"Dharini Chandrasekar","email":"dharini.chandrasekar@intel.com","username":"dharinic"},"change_message_id":"d1a49d913f394e4c2d66381b6ba686fb592133e2","unresolved":false,"context_lines":[{"line_number":258,"context_line":"        :param image_id: Glance Image ID"},{"line_number":259,"context_line":"        \"\"\""},{"line_number":260,"context_line":"        new_image \u003d self.image_repo.get(image_id)"},{"line_number":261,"context_line":"        if new_image.status \u003d\u003d \u0027saving\u0027:"},{"line_number":262,"context_line":"            # NOTE(flaper87): THIS IS WRONG!"},{"line_number":263,"context_line":"            # we should be doing atomic updates to avoid"},{"line_number":264,"context_line":"            # race conditions. This happens in other places"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a1ced50_1a63e01b","line":261,"range":{"start_line":261,"start_character":32,"end_line":261,"end_character":38},"updated":"2017-03-17 18:58:56.000000000","message":"s/saving/importing","commit_id":"530336b1983a2a828c77a03fdc1d730a87bd7ade"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"7e053749397f6490a608f70606b9c960f9e0e174","unresolved":false,"context_lines":[{"line_number":263,"context_line":"            # we should be doing atomic updates to avoid"},{"line_number":264,"context_line":"            # race conditions. This happens in other places"},{"line_number":265,"context_line":"            # too."},{"line_number":266,"context_line":"            new_image.status \u003d \u0027active\u0027"},{"line_number":267,"context_line":"        self.image_repo.save(new_image)"},{"line_number":268,"context_line":""},{"line_number":269,"context_line":""}],"source_content_type":"text/x-python","patch_set":5,"id":"1a1ced50_d9e55aa4","line":266,"updated":"2017-03-22 00:11:17.000000000","message":"why not just use new_image.image_update with the correct from_state to set the new status so we don\u0027t have a race condition?","commit_id":"530336b1983a2a828c77a03fdc1d730a87bd7ade"},{"author":{"_account_id":12807,"name":"Steve Lewis (stevelle)","email":"stevelle@gmail.com","username":"stevelle"},"change_message_id":"7e053749397f6490a608f70606b9c960f9e0e174","unresolved":false,"context_lines":[{"line_number":326,"context_line":"    uri \u003d kwargs.get(\u0027uri\u0027)"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    if not uri:"},{"line_number":329,"context_line":"        separator \u003d \u0027\u0027"},{"line_number":330,"context_line":"        if not CONF.node_staging_uri.endsWith(\u0027/\u0027):"},{"line_number":331,"context_line":"            separator \u003d \u0027/\u0027"},{"line_number":332,"context_line":"        uri \u003d separator.join((CONF.node_staging_uri, str(image_id)))"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"    flow \u003d lf.flow(task_type, retry\u003dretry.AlwaysRevert())"},{"line_number":335,"context_line":"    flow.add(_VerifyStaging(task_id, task_type, uri))"}],"source_content_type":"text/x-python","patch_set":5,"id":"1a1ced50_7432adf3","line":332,"range":{"start_line":329,"start_character":8,"end_line":332,"end_character":68},"updated":"2017-03-22 00:11:17.000000000","message":"why not use urljoin?\n\n  urlparse.urljoin(CONF.node_staging_uri, str(image_id))","commit_id":"530336b1983a2a828c77a03fdc1d730a87bd7ade"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"67b22b57966c8326062271c9b9361c7dc82870d1","unresolved":false,"context_lines":[{"line_number":35,"context_line":"CONF \u003d cfg.CONF"},{"line_number":36,"context_line":""},{"line_number":37,"context_line":""},{"line_number":38,"context_line":"# TODO(jokke): We should refactor the task implementations so that we do not"},{"line_number":39,"context_line":"# need to duplicate what we have already for example in base_import.py."},{"line_number":40,"context_line":""},{"line_number":41,"context_line":"class _DeleteFromFS(task.Task):"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff0f0b1f_e953256a","line":38,"updated":"2017-05-23 07:37:48.000000000","message":"Do you mean re-organize where the classes are and their names (remove the _ prefix)? Asking because you should be able to re-use tasks, unless I\u0027m missing something","commit_id":"fe016e727360f51bd05b0000759f541c7e5ff13c"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"67b22b57966c8326062271c9b9361c7dc82870d1","unresolved":false,"context_lines":[{"line_number":72,"context_line":"        # NOTE(jokke): If we want to use other than \u0027file\u0027 store in the"},{"line_number":73,"context_line":"        # future, this is one thing that needs to change."},{"line_number":74,"context_line":"        try:"},{"line_number":75,"context_line":"            uri.index(\u0027file:///\u0027, 0)"},{"line_number":76,"context_line":"        except ValueError:"},{"line_number":77,"context_line":"            msg \u003d (_(\"%(task_id)s of %(task_type)s not configured \""},{"line_number":78,"context_line":"                     \"properly. Value of node_staging_uri must be \""}],"source_content_type":"text/x-python","patch_set":8,"id":"ff0f0b1f_69f6f565","line":75,"updated":"2017-05-23 07:37:48.000000000","message":"nitpick: Using `startswith` would have been clearer.","commit_id":"fe016e727360f51bd05b0000759f541c7e5ff13c"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"67b22b57966c8326062271c9b9361c7dc82870d1","unresolved":false,"context_lines":[{"line_number":326,"context_line":"    uri \u003d kwargs.get(\u0027uri\u0027)"},{"line_number":327,"context_line":""},{"line_number":328,"context_line":"    if not uri:"},{"line_number":329,"context_line":"        separator \u003d \u0027\u0027"},{"line_number":330,"context_line":"        if not CONF.node_staging_uri.endsWith(\u0027/\u0027):"},{"line_number":331,"context_line":"            separator \u003d \u0027/\u0027"},{"line_number":332,"context_line":"        uri \u003d separator.join((CONF.node_staging_uri, str(image_id)))"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"    flow \u003d lf.flow(task_type, retry\u003dretry.AlwaysRevert())"},{"line_number":335,"context_line":"    flow.add(_VerifyStaging(task_id, task_type, uri))"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff0f0b1f_29ba3d6d","line":332,"range":{"start_line":329,"start_character":0,"end_line":332,"end_character":68},"updated":"2017-05-23 07:37:48.000000000","message":"nit: fwiw, os.path.join should be able to do all this","commit_id":"fe016e727360f51bd05b0000759f541c7e5ff13c"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"67b22b57966c8326062271c9b9361c7dc82870d1","unresolved":false,"context_lines":[{"line_number":331,"context_line":"            separator \u003d \u0027/\u0027"},{"line_number":332,"context_line":"        uri \u003d separator.join((CONF.node_staging_uri, str(image_id)))"},{"line_number":333,"context_line":""},{"line_number":334,"context_line":"    flow \u003d lf.flow(task_type, retry\u003dretry.AlwaysRevert())"},{"line_number":335,"context_line":"    flow.add(_VerifyStaging(task_id, task_type, uri))"},{"line_number":336,"context_line":""},{"line_number":337,"context_line":"    # TODO(jokke): For the pluggable tasks like image verification or"}],"source_content_type":"text/x-python","patch_set":8,"id":"ff0f0b1f_293d9ddc","line":334,"updated":"2017-05-23 07:37:48.000000000","message":"It\u0027d be useful to document that, for now, none of the tasks above implement a revert logic. This is something that we might want to revisit in the future.","commit_id":"fe016e727360f51bd05b0000759f541c7e5ff13c"}]}
