)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"3911f4fa912cf30cdff22b4cd13ddbaad9ada452","unresolved":false,"context_lines":[{"line_number":17,"context_line":"The implementation uses stevedore to load both, import and optional"},{"line_number":18,"context_line":"flows, and chains them using linear and unordered flows where needed."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Partially-implements blueprint: introspection-of-images"},{"line_number":23,"context_line":"Change-Id: I9e7505d4e84aabf9d71e03c360514ac824ea84de"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"ba7be1f8_f7bf58a8","line":20,"updated":"2015-02-23 15:41:46.000000000","message":"SecurityImpact?\n\nDo we have any concerns?","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"deefe1b0882de391ba804520d883f7212cae5e43","unresolved":false,"context_lines":[{"line_number":17,"context_line":"The implementation uses stevedore to load both, import and optional"},{"line_number":18,"context_line":"flows, and chains them using linear and unordered flows where needed."},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Partially-implements blueprint: introspection-of-images"},{"line_number":23,"context_line":"Change-Id: I9e7505d4e84aabf9d71e03c360514ac824ea84de"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"ba7be1f8_4c97a890","line":20,"in_reply_to":"ba7be1f8_f7bf58a8","updated":"2015-02-23 23:39:14.000000000","message":"none comes to mind but I\u0027ll happily add it if there are some","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"3911f4fa912cf30cdff22b4cd13ddbaad9ada452","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Partially-implements blueprint: introspection-of-images"},{"line_number":23,"context_line":"Change-Id: I9e7505d4e84aabf9d71e03c360514ac824ea84de"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"ba7be1f8_f7c3d854","line":22,"updated":"2015-02-23 15:41:46.000000000","message":"how about adding partial: https://blueprints.launchpad.net/glance/+spec/new-upload-workflow and/or partial https://blueprints.launchpad.net/glance/+spec/basic-import-conversion","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"deefe1b0882de391ba804520d883f7212cae5e43","unresolved":false,"context_lines":[{"line_number":19,"context_line":""},{"line_number":20,"context_line":"DocImpact"},{"line_number":21,"context_line":""},{"line_number":22,"context_line":"Partially-implements blueprint: introspection-of-images"},{"line_number":23,"context_line":"Change-Id: I9e7505d4e84aabf9d71e03c360514ac824ea84de"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":8,"id":"ba7be1f8_4cdec86e","line":22,"in_reply_to":"ba7be1f8_f7c3d854","updated":"2015-02-23 23:39:14.000000000","message":"The image conversion is coming in a separate patch, I\u0027ll add a line for the upload-workflow","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"}],"doc/source/configuring.rst":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d8cc4cd9942273da4c5a95062e9f7331f347b2b4","unresolved":false,"context_lines":[{"line_number":1375,"context_line":"instantiated per executor instance."},{"line_number":1376,"context_line":""},{"line_number":1377,"context_line":"The default value for the ``engine_mode`` is ``worker-based``, whereas"},{"line_number":1378,"context_line":"the default number of ``max_workers`` is ``2``."},{"line_number":1379,"context_line":""},{"line_number":1380,"context_line":"Configuring Glance performance profiling"},{"line_number":1381,"context_line":"----------------------------------------"}],"source_content_type":"text/x-rst","patch_set":13,"id":"ba7be1f8_dad5ade5","line":1378,"updated":"2015-02-25 17:09:18.000000000","message":"You changed this to 50 instead of 2 and reverted engine_mode back to parallel.","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"d9d349eebe6922361b38e0790b30c9a45f8827e0","unresolved":false,"context_lines":[{"line_number":1375,"context_line":"instantiated per executor instance."},{"line_number":1376,"context_line":""},{"line_number":1377,"context_line":"The default value for the ``engine_mode`` is ``worker-based``, whereas"},{"line_number":1378,"context_line":"the default number of ``max_workers`` is ``2``."},{"line_number":1379,"context_line":""},{"line_number":1380,"context_line":"Configuring Glance performance profiling"},{"line_number":1381,"context_line":"----------------------------------------"}],"source_content_type":"text/x-rst","patch_set":13,"id":"ba7be1f8_6a86b0dc","line":1378,"in_reply_to":"ba7be1f8_dad5ade5","updated":"2015-02-25 18:23:01.000000000","message":"will fix","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"}],"etc/glance-api.conf":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"3911f4fa912cf30cdff22b4cd13ddbaad9ada452","unresolved":false,"context_lines":[{"line_number":466,"context_line":""},{"line_number":467,"context_line":"# The number of parallel activities executed at the same time by"},{"line_number":468,"context_line":"# the engine. The value can be greater than one when the engine mode is"},{"line_number":469,"context_line":"# \u0027worker-based\u0027."},{"line_number":470,"context_line":"#max_workers \u003d 1"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"[glance_store]"}],"source_content_type":"text/plain","patch_set":8,"id":"ba7be1f8_bc84e7b1","line":469,"updated":"2015-02-23 15:41:46.000000000","message":"Where did the parallel disappear?","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":8984,"name":"George Peristerakis","email":"gperiste@redhat.com","username":"peristeri"},"change_message_id":"5127154ea6da536d5853d6754bd1da8407d896d5","unresolved":false,"context_lines":[{"line_number":459,"context_line":"# eventlet_executor_pool_size \u003d 1000"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"[taskflow_executor]"},{"line_number":462,"context_line":"# The mode in which the engine will run. Can be \u0027default, \u0027serial\u0027,"},{"line_number":463,"context_line":"# \u0027parallel\u0027 or \u0027worker-based\u0027"},{"line_number":464,"context_line":"#engine_mode \u003d serial"},{"line_number":465,"context_line":""}],"source_content_type":"text/plain","patch_set":13,"id":"ba7be1f8_063e168c","line":462,"updated":"2015-02-25 16:50:15.000000000","message":"You are missing a quote on default","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"7e6c96d030d4ed530e5ed6c35213f027f5ceb98a","unresolved":false,"context_lines":[{"line_number":459,"context_line":"# eventlet_executor_pool_size \u003d 1000"},{"line_number":460,"context_line":""},{"line_number":461,"context_line":"[taskflow_executor]"},{"line_number":462,"context_line":"# The mode in which the engine will run. Can be \u0027default, \u0027serial\u0027,"},{"line_number":463,"context_line":"# \u0027parallel\u0027 or \u0027worker-based\u0027"},{"line_number":464,"context_line":"#engine_mode \u003d serial"},{"line_number":465,"context_line":""}],"source_content_type":"text/plain","patch_set":13,"id":"ba7be1f8_9ac89556","line":462,"in_reply_to":"ba7be1f8_063e168c","updated":"2015-02-25 17:08:56.000000000","message":"ops, thanks","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"77ec8e32ae08fba2ffda7ae702a4eea538675cd2","unresolved":false,"context_lines":[{"line_number":482,"context_line":"# eventlet_executor_pool_size \u003d 1000"},{"line_number":483,"context_line":""},{"line_number":484,"context_line":"[taskflow_executor]"},{"line_number":485,"context_line":"# The mode in which the engine will run. Can bedefault\u0027, \u0027serial\u0027,"},{"line_number":486,"context_line":"# \u0027parallel\u0027 or \u0027worker-based\u0027"},{"line_number":487,"context_line":"#engine_mode \u003d serial"},{"line_number":488,"context_line":""}],"source_content_type":"text/plain","patch_set":24,"id":"9a80dd14_0066c1ba","line":485,"updated":"2015-03-10 14:00:30.000000000","message":"I think you meant\n\n    Can be \u0027default\u0027","commit_id":"0503fdc2d81097d9453ce6ad844785fbb7905b79"}],"glance/api/middleware/context.py":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"3911f4fa912cf30cdff22b4cd13ddbaad9ada452","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        \"\"\"Create a context without an authorized user.\"\"\""},{"line_number":130,"context_line":"        kwargs \u003d {"},{"line_number":131,"context_line":"            \u0027user\u0027: None,"},{"line_number":132,"context_line":"            \u0027tenant\u0027: \u0027test\u0027,"},{"line_number":133,"context_line":"            \u0027roles\u0027: [],"},{"line_number":134,"context_line":"            \u0027is_admin\u0027: True,"},{"line_number":135,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_bc7227e1","line":132,"updated":"2015-02-23 15:41:46.000000000","message":"Why?","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"deefe1b0882de391ba804520d883f7212cae5e43","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        \"\"\"Create a context without an authorized user.\"\"\""},{"line_number":130,"context_line":"        kwargs \u003d {"},{"line_number":131,"context_line":"            \u0027user\u0027: None,"},{"line_number":132,"context_line":"            \u0027tenant\u0027: \u0027test\u0027,"},{"line_number":133,"context_line":"            \u0027roles\u0027: [],"},{"line_number":134,"context_line":"            \u0027is_admin\u0027: True,"},{"line_number":135,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_d6efa585","line":132,"in_reply_to":"ba7be1f8_bc7227e1","updated":"2015-02-23 23:39:14.000000000","message":"erm, un-related change. Will fix","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"baa70e92f7b808f93a610bcd06b4d0506cc871da","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        kwargs \u003d {"},{"line_number":132,"context_line":"            \u0027user\u0027: req.headers.get(\u0027X-User-Id\u0027),"},{"line_number":133,"context_line":"            \u0027tenant\u0027: req.headers.get(\u0027X-User-Id\u0027),"},{"line_number":134,"context_line":"            \u0027roles\u0027: [],"},{"line_number":135,"context_line":"            \u0027is_admin\u0027: True,"},{"line_number":136,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_cd5b48f4","line":133,"updated":"2015-02-25 13:42:45.000000000","message":"Shouldn\u0027t this be \u0027X-Tenant-Id\u0027?","commit_id":"e4e0d47f523663f2501879f5367c3fdaf8bd418b"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"f1c525127d5c29ce363c8b70a8ce31edb8b0ff72","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        kwargs \u003d {"},{"line_number":132,"context_line":"            \u0027user\u0027: req.headers.get(\u0027X-User-Id\u0027),"},{"line_number":133,"context_line":"            \u0027tenant\u0027: req.headers.get(\u0027X-User-Id\u0027),"},{"line_number":134,"context_line":"            \u0027roles\u0027: [],"},{"line_number":135,"context_line":"            \u0027is_admin\u0027: True,"},{"line_number":136,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_26b7a13b","line":133,"in_reply_to":"ba7be1f8_cd5b48f4","updated":"2015-02-25 14:07:23.000000000","message":"lol, fuck copy/paste","commit_id":"e4e0d47f523663f2501879f5367c3fdaf8bd418b"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        \"\"\"Create a context without an authorized user.\"\"\""},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        kwargs \u003d {"},{"line_number":132,"context_line":"            \u0027user\u0027: req.headers.get(\u0027X-User-Id\u0027),"},{"line_number":133,"context_line":"            \u0027tenant\u0027: req.headers.get(\u0027X-Tenant-Id\u0027),"},{"line_number":134,"context_line":"            \u0027roles\u0027: [],"},{"line_number":135,"context_line":"            \u0027is_admin\u0027: True,"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_606fba2b","line":132,"updated":"2015-03-03 23:34:28.000000000","message":"If the context is \"Unauthenticated\", why are we changing this?","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":129,"context_line":"        \"\"\"Create a context without an authorized user.\"\"\""},{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        kwargs \u003d {"},{"line_number":132,"context_line":"            \u0027user\u0027: req.headers.get(\u0027X-User-Id\u0027),"},{"line_number":133,"context_line":"            \u0027tenant\u0027: req.headers.get(\u0027X-Tenant-Id\u0027),"},{"line_number":134,"context_line":"            \u0027roles\u0027: [],"},{"line_number":135,"context_line":"            \u0027is_admin\u0027: True,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_f52ebba4","line":132,"in_reply_to":"ba7be1f8_606fba2b","updated":"2015-03-04 13:52:36.000000000","message":"because in order to create a task, you need to have a tenant. For unauthenticated contexts (or testing) it\u0027s useful to have a way to pass this things in the header instead of hard-coding dummy tenants here.","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"a88c98d0af38aa8bee80056a2e501e6869955f4d","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        kwargs \u003d {"},{"line_number":132,"context_line":"            \u0027user\u0027: req.headers.get(\u0027X-User-Id\u0027),"},{"line_number":133,"context_line":"            \u0027tenant\u0027: req.headers.get(\u0027X-Tenant-Id\u0027),"},{"line_number":134,"context_line":"            \u0027roles\u0027: [],"},{"line_number":135,"context_line":"            \u0027is_admin\u0027: True,"},{"line_number":136,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a80dd14_f0ec16f6","line":133,"updated":"2015-03-10 14:56:08.000000000","message":"I\u0027ve questions around making this change here. This may change the behavior of deployments using UnAuth middleware.","commit_id":"0503fdc2d81097d9453ce6ad844785fbb7905b79"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"5f7ff5a2387e0ddacb4ef380e6b06ab7724ed0ee","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        kwargs \u003d {"},{"line_number":132,"context_line":"            \u0027user\u0027: req.headers.get(\u0027X-User-Id\u0027),"},{"line_number":133,"context_line":"            \u0027tenant\u0027: req.headers.get(\u0027X-Tenant-Id\u0027),"},{"line_number":134,"context_line":"            \u0027roles\u0027: [],"},{"line_number":135,"context_line":"            \u0027is_admin\u0027: True,"},{"line_number":136,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a80dd14_a13fb2b4","line":133,"in_reply_to":"9a80dd14_76b81ee6","updated":"2015-03-10 15:57:08.000000000","message":"noauth can be useful in numerous cases like admin nodes, noauth registry sitting behind auth based g-api, etc. There are real world use cases so, I think we need to respect them.\n\nAgree to fact that config seems like the right approach here. We can do that outside of the BP.","commit_id":"0503fdc2d81097d9453ce6ad844785fbb7905b79"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"7a74ce3c87b7eff32eed12bf5583eb8120cfaf9b","unresolved":false,"context_lines":[{"line_number":130,"context_line":""},{"line_number":131,"context_line":"        kwargs \u003d {"},{"line_number":132,"context_line":"            \u0027user\u0027: req.headers.get(\u0027X-User-Id\u0027),"},{"line_number":133,"context_line":"            \u0027tenant\u0027: req.headers.get(\u0027X-Tenant-Id\u0027),"},{"line_number":134,"context_line":"            \u0027roles\u0027: [],"},{"line_number":135,"context_line":"            \u0027is_admin\u0027: True,"},{"line_number":136,"context_line":"        }"}],"source_content_type":"text/x-python","patch_set":24,"id":"9a80dd14_76b81ee6","line":133,"in_reply_to":"9a80dd14_f0ec16f6","updated":"2015-03-10 15:28:03.000000000","message":"mmh, I don\u0027t think this is bad. I believe, as mentioned on IRC, using this unauth middleware in production is actually bad. Without the information of what the image user/owner is, bad things could happen. You need to either use this middleware everywhere or not use it at all. Otherwise, you\u0027ll end-up into authorization issues, AFAICT.\n\nThe reason behind this change is that tasks require a tenant to work properly.\n\nThat said, I\u0027ll revert this change, I guess we could add some config options to make this thing work.","commit_id":"0503fdc2d81097d9453ce6ad844785fbb7905b79"}],"glance/async/flows/base_import.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a6015f19d8bb8e781e8dc147787a3d8a899f5302","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # later. The most important point here is to re-use the existing code"},{"line_number":97,"context_line":"        # in glance_store."},{"line_number":98,"context_line":"        if \u0027file\u0027 not in store_api.get_known_schemes():"},{"line_number":99,"context_line":"            raise RuntimeError(\"We need the filesystem store\")"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def execute(self, image):"},{"line_number":102,"context_line":"        store \u003d store_api.get_store_from_scheme(\u0027file\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_424eac97","line":99,"updated":"2015-02-23 12:14:50.000000000","message":"Shouldn\u0027t we specify in exception message who needed the store?\nIn this case it could be \"import extensions\"","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"01e8f2c1be2e2dad64cb73af1f4910907309bf9f","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # later. The most important point here is to re-use the existing code"},{"line_number":97,"context_line":"        # in glance_store."},{"line_number":98,"context_line":"        if \u0027file\u0027 not in store_api.get_known_schemes():"},{"line_number":99,"context_line":"            raise RuntimeError(\"We need the filesystem store\")"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def execute(self, image):"},{"line_number":102,"context_line":"        store \u003d store_api.get_store_from_scheme(\u0027file\u0027)"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_1b26b6ce","line":99,"in_reply_to":"ba7be1f8_424eac97","updated":"2015-02-24 10:01:46.000000000","message":"yeah, I gotta improve the message here. I mean, I\u0027d like to log a proper message and leave a dummy message in the `RuntimeException` as I\u0027m afraid it\u0027ll be sent back to the user.","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a6015f19d8bb8e781e8dc147787a3d8a899f5302","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                                              context\u003dNone)[0]"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        return path"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"class _DeleteFromFS(task.Task):"},{"line_number":114,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_85726eab","line":111,"updated":"2015-02-23 12:14:50.000000000","message":"Shouldn\u0027t we have a revert method to delete the image?\nOr am I mistaken in believing that the linear flow doesn\u0027t revert on error by default and ignores it instead?\nBecause if it ignores and keeps going forward it\u0027s fine because it will reach _DeleteFromFS, but if it reverts, the image will never get removed.","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"01e8f2c1be2e2dad64cb73af1f4910907309bf9f","unresolved":false,"context_lines":[{"line_number":108,"context_line":"                                              context\u003dNone)[0]"},{"line_number":109,"context_line":""},{"line_number":110,"context_line":"        return path"},{"line_number":111,"context_line":""},{"line_number":112,"context_line":""},{"line_number":113,"context_line":"class _DeleteFromFS(task.Task):"},{"line_number":114,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_5e1e5c64","line":111,"in_reply_to":"ba7be1f8_85726eab","updated":"2015-02-24 10:01:46.000000000","message":"yes, we actually need a revert method. Good catch, again :D","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a6015f19d8bb8e781e8dc147787a3d8a899f5302","unresolved":false,"context_lines":[{"line_number":218,"context_line":"    # the duplicated calls to those tasks by creating a linear flow that"},{"line_number":219,"context_line":"    # ensures those are called before the other tasks."},{"line_number":220,"context_line":"    # For now, I\u0027m keeping them here, though."},{"line_number":221,"context_line":"    import_flows \u003d lf.Flow(task_type).add(_ImportToFS(task_id,"},{"line_number":222,"context_line":"                                                      task_type,"},{"line_number":223,"context_line":"                                                      task_repo,"},{"line_number":224,"context_line":"                                                      uri))"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_c220dcb2","line":221,"updated":"2015-02-23 12:14:50.000000000","message":"This would look better after checking that we have extensions on line 231","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"01e8f2c1be2e2dad64cb73af1f4910907309bf9f","unresolved":false,"context_lines":[{"line_number":218,"context_line":"    # the duplicated calls to those tasks by creating a linear flow that"},{"line_number":219,"context_line":"    # ensures those are called before the other tasks."},{"line_number":220,"context_line":"    # For now, I\u0027m keeping them here, though."},{"line_number":221,"context_line":"    import_flows \u003d lf.Flow(task_type).add(_ImportToFS(task_id,"},{"line_number":222,"context_line":"                                                      task_type,"},{"line_number":223,"context_line":"                                                      task_repo,"},{"line_number":224,"context_line":"                                                      uri))"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_fe2470b0","line":221,"in_reply_to":"ba7be1f8_c220dcb2","updated":"2015-02-24 10:01:46.000000000","message":"this changed in the last PS","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a6015f19d8bb8e781e8dc147787a3d8a899f5302","unresolved":false,"context_lines":[{"line_number":230,"context_line":"    if len(extensions.extensions) \u003d\u003d 0:"},{"line_number":231,"context_line":"        return None"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    for ext in extensions.extensions:"},{"line_number":234,"context_line":"        import_flows.add(ext.obj)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    return import_flows.add(_DeleteFromFS(task_id, task_type))"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_650ada22","line":233,"updated":"2015-02-23 12:14:50.000000000","message":"This should be an unordered flow inside the ordered flow, since extensions can be done in parallel.","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"01e8f2c1be2e2dad64cb73af1f4910907309bf9f","unresolved":false,"context_lines":[{"line_number":230,"context_line":"    if len(extensions.extensions) \u003d\u003d 0:"},{"line_number":231,"context_line":"        return None"},{"line_number":232,"context_line":""},{"line_number":233,"context_line":"    for ext in extensions.extensions:"},{"line_number":234,"context_line":"        import_flows.add(ext.obj)"},{"line_number":235,"context_line":""},{"line_number":236,"context_line":"    return import_flows.add(_DeleteFromFS(task_id, task_type))"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_3e3608e8","line":233,"in_reply_to":"ba7be1f8_650ada22","updated":"2015-02-24 10:01:46.000000000","message":"this changed in the last PS","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"3911f4fa912cf30cdff22b4cd13ddbaad9ada452","unresolved":false,"context_lines":[{"line_number":94,"context_line":"        # may have a performance impact on the access to the image. Still, it"},{"line_number":95,"context_line":"        # doesn\u0027t seem to be a big issue for now and we can refactor this code"},{"line_number":96,"context_line":"        # later. The most important point here is to re-use the existing code"},{"line_number":97,"context_line":"        # in glance_store."},{"line_number":98,"context_line":"        if \u0027file\u0027 not in store_api.get_known_schemes():"},{"line_number":99,"context_line":"            raise RuntimeError(\"We need the filesystem store\")"},{"line_number":100,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_372ad07e","line":97,"updated":"2015-02-23 15:41:46.000000000","message":"Could we utilize the cache area instead rather than FS-Store for this? That would ensure us staying on the local node.","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"3911f4fa912cf30cdff22b4cd13ddbaad9ada452","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # later. The most important point here is to re-use the existing code"},{"line_number":97,"context_line":"        # in glance_store."},{"line_number":98,"context_line":"        if \u0027file\u0027 not in store_api.get_known_schemes():"},{"line_number":99,"context_line":"            raise RuntimeError(\"We need the filesystem store\")"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def execute(self, image_id):"},{"line_number":102,"context_line":"        store \u003d store_api.get_store_from_scheme(\u0027file\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_e91b001a","line":99,"updated":"2015-02-23 15:41:46.000000000","message":"Emphatising my previous comment. Feels really wrong to have a need for FS store to perform basic functionality like this. If we want to keep doing this I think we should make possible to configure FS store just for the tasks (and give possibility to set tasks using in-use FS store, but not demanding the use of the Glance wide FS store).","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"3e5db15d7e47956a60ad8c3298e42416a3d1877e","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # later. The most important point here is to re-use the existing code"},{"line_number":97,"context_line":"        # in glance_store."},{"line_number":98,"context_line":"        if \u0027file\u0027 not in store_api.get_known_schemes():"},{"line_number":99,"context_line":"            raise RuntimeError(\"We need the filesystem store\")"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def execute(self, image_id):"},{"line_number":102,"context_line":"        store \u003d store_api.get_store_from_scheme(\u0027file\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_52a36bb1","line":99,"in_reply_to":"ba7be1f8_30d99722","updated":"2015-02-25 15:11:52.000000000","message":"I\u0027ve put some extra thoughts on this and took the chance to brainstorm offline with other folks. I think we can achieve what we both want by adding a new configuration option as you suggested and checking if the FS path is the same as the \"scratch path\" one.\n\nHere are some scenarios, all the scenarios below assume the user is using a FS store as the default store:\n\nScenario 1: FS Store is Remote, introspection enabled, conversion disabled\n\nIn this scenario, the user would benefit from having the scratch path being the same path as the fs store. Only one write would happen and an extra read will happen in order to introspect the image. Note that this read is just for the image headers and not the entire file.\n\nScenario 2: FS Store is remote, introspection enabled, conversion enabled\n\nIn this scenario, the user would benefit from having a *local* store into which the image can be converted. This will required downloading the image locally, converting it and then copying the converted image to the remote store.\n\nScenario 3: FS Store is local, introspection enabled, conversion disabled\nScenario 4: FS Store is local, introspection enabled, conversion enabled\n\nIn both this scenarios the user shouldn\u0027t care, therefore probably benefit, about the scratch path and the FS store being the same from a performance perspective. Space wise, regardless of the scenario, the user will have to account for it in advance.\n\nNote that any of the scenarios above, especially the ones regarding local fs store, take into account the disk performance. It may be desirable to have dedicate disks for image conversions.\n\nIn summary, in the worst case scenario, the image will be read 3 times and written twice, whereas in the best case scenario, the image will be read and written once.","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"0ac99c773dcfc1d7f7b75cb096cde68f5d9b161d","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # later. The most important point here is to re-use the existing code"},{"line_number":97,"context_line":"        # in glance_store."},{"line_number":98,"context_line":"        if \u0027file\u0027 not in store_api.get_known_schemes():"},{"line_number":99,"context_line":"            raise RuntimeError(\"We need the filesystem store\")"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def execute(self, image_id):"},{"line_number":102,"context_line":"        store \u003d store_api.get_store_from_scheme(\u0027file\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_a3761c04","line":99,"in_reply_to":"ba7be1f8_52a36bb1","updated":"2015-02-25 15:55:10.000000000","message":"Here\u0027s my two cents.\n\nI think it is a good idea to have a \"scratch path\" and use it for the import extensions that requires it (introspection and conversion) instead of using production file storage.\n\nIt would be great to make the flow smart enough to detect when the FS path and Scratch path are the same to avoid writing multiple times the same image (if there\u0027s no conversion).\n\nAnd I would say that if no scratch path is configured all import extensions would be automatically disabled, since they cannot work without a scratch path.","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"b12c887d89f4bee887966d833f954dd1d5a87bdb","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # later. The most important point here is to re-use the existing code"},{"line_number":97,"context_line":"        # in glance_store."},{"line_number":98,"context_line":"        if \u0027file\u0027 not in store_api.get_known_schemes():"},{"line_number":99,"context_line":"            raise RuntimeError(\"We need the filesystem store\")"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def execute(self, image_id):"},{"line_number":102,"context_line":"        store \u003d store_api.get_store_from_scheme(\u0027file\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_30d99722","line":99,"in_reply_to":"ba7be1f8_8cba8021","updated":"2015-02-25 13:42:55.000000000","message":"To be honest I\u0027d rather have those extra config options than assume or force anything. Complex environments are complex to configure as well, but it\u0027s better than fecking everything up.\n\nAlso I do not like the idea that we poke the \"production\" store while doing these tasks. The benefit of having the data on the correct store already are really specific for that single use case where deployer happens to have default FS store in use and as you said we can\u0027t even take the benefit of it yet.\n\nI\u0027m fine using the store code and FS store for this and perhaps we could implement it in such way that if the FS store for this is configured with the same value as existing store we can expand the \"single import\" for those who wish to be adventurous. Lets just not force it upon everyone.","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"f460bfb8550e9bc80f6df9483e069e3c260524ea","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # later. The most important point here is to re-use the existing code"},{"line_number":97,"context_line":"        # in glance_store."},{"line_number":98,"context_line":"        if \u0027file\u0027 not in store_api.get_known_schemes():"},{"line_number":99,"context_line":"            raise RuntimeError(\"We need the filesystem store\")"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def execute(self, image_id):"},{"line_number":102,"context_line":"        store \u003d store_api.get_store_from_scheme(\u0027file\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_94494c33","line":99,"in_reply_to":"ba7be1f8_a3761c04","updated":"2015-02-25 16:33:35.000000000","message":"yup, I think we\u0027re on the same page.\n\nI wouldn\u0027t say *all* import extensions need to be disabled but the ones that depend on that path.","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"deefe1b0882de391ba804520d883f7212cae5e43","unresolved":false,"context_lines":[{"line_number":96,"context_line":"        # later. The most important point here is to re-use the existing code"},{"line_number":97,"context_line":"        # in glance_store."},{"line_number":98,"context_line":"        if \u0027file\u0027 not in store_api.get_known_schemes():"},{"line_number":99,"context_line":"            raise RuntimeError(\"We need the filesystem store\")"},{"line_number":100,"context_line":""},{"line_number":101,"context_line":"    def execute(self, image_id):"},{"line_number":102,"context_line":"        store \u003d store_api.get_store_from_scheme(\u0027file\u0027)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_8cba8021","line":99,"in_reply_to":"ba7be1f8_e91b001a","updated":"2015-02-23 23:39:14.000000000","message":"Yes, I fully agree with this. Note that this part was still open for discussion.\n\nBefore we go through what\u0027s correct and what\u0027s wrong, lets analyze what we need to make this thing work.\n\nWe need some space, which we can\u0027t estimate in advance, that will allow us to run certain commands over the image data. In the case of the image introspection that command is `qemu-img info` and in the case of image conversion it\u0027s `qemu-img convert`.\n\nWe currently have some config options that take paths to filesystem directories. One of those is the cache area, the other is the filesystem store (when configured).\n\nUsing the cache area feels as bad as using the FS store. I\u0027d dare to say that I\u0027d rather use the FS store than the cache area because the FS store is supposed to do exactly this, store data. Nonetheless, it\u0027d probably be better to have a separate config option for this.\n\nThe main reason I didn\u0027t go with that is that I\u0027d prefer not adding yet-another-option and more importantly yet-another-thing the operator has to account for in advance and that will make future upgrades harder. As mentioned in my comment in the code, a separate store doesn\u0027t buy us much. I\u0027ll try to extend that comment a bit more here:\n\nLets start with the, perhaps, easier case. If the FS store is not being used as the default store, it\u0027d be fine - I reckon - to ask the operator to configure such store for these asynchronous tasks. The store could either be configured locally or in a remote NFS. Whatever the operator prefers.\n\nIf instead the FS store is being used as the default store, we do have a somewhat not-ideal scenario because we\u0027ll be hitting the main store for asynchronous task\u0027s operations. However, we have an advantage, which is that we can play smart and avoid importing the file \"twice\" into the store because we know it\u0027s there already. The current code is not *THAT* smart but all this is in the works.\n\nIt\u0027ll be hard to get it all right in a single patch because the way our tasks work right now is far from perfect and this is something we\u0027re working on fixing now. We\u0027ll improve the API and the interaction as we add new tasks to the process.\n\nOne more thing I think favors the usage of the FS store -at least for now- is that it\u0027s easier to add an option later than removing it.\n\nI\u0027m definitely not 100% opposed to adding specific FS options for this but, I\u0027d say that I prefer not to and definitely I\u0027d like to re-use our glance_store code to handle this instead of writing yet-another-file-handler code for this.\n\nthoughts?","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"3911f4fa912cf30cdff22b4cd13ddbaad9ada452","unresolved":false,"context_lines":[{"line_number":194,"context_line":"            # necessary"},{"line_number":195,"context_line":"            err_msg \u003d (\"Error: \" + six.text_type(type(e)) + \u0027: \u0027 +"},{"line_number":196,"context_line":"                       common_utils.exception_to_str(e))"},{"line_number":197,"context_line":"            log_msg \u003d _LE(err_msg + (\"Task ID %s\" % task.task_id))  # noqa"},{"line_number":198,"context_line":"            LOG.exception(log_msg)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"            task.fail(_LE(err_msg))  # noqa"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_f7011807","line":197,"updated":"2015-02-23 15:41:46.000000000","message":"NIT: \" Task ID %s\" We probably won\u0027t have white space at the end of the string.","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"3911f4fa912cf30cdff22b4cd13ddbaad9ada452","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            log_msg \u003d _LE(err_msg + (\"Task ID %s\" % task.task_id))  # noqa"},{"line_number":198,"context_line":"            LOG.exception(log_msg)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"            task.fail(_LE(err_msg))  # noqa"},{"line_number":201,"context_line":"        finally:"},{"line_number":202,"context_line":"            self.task_repo.save(task)"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_4927acb6","line":200,"updated":"2015-02-23 15:41:46.000000000","message":"Why #noqa?","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"deefe1b0882de391ba804520d883f7212cae5e43","unresolved":false,"context_lines":[{"line_number":197,"context_line":"            log_msg \u003d _LE(err_msg + (\"Task ID %s\" % task.task_id))  # noqa"},{"line_number":198,"context_line":"            LOG.exception(log_msg)"},{"line_number":199,"context_line":""},{"line_number":200,"context_line":"            task.fail(_LE(err_msg))  # noqa"},{"line_number":201,"context_line":"        finally:"},{"line_number":202,"context_line":"            self.task_repo.save(task)"},{"line_number":203,"context_line":""}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_d6640524","line":200,"in_reply_to":"ba7be1f8_4927acb6","updated":"2015-02-23 23:39:14.000000000","message":"no idea","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"7278298a5dcc28c4fb58ee5b5dad90fd2798972c","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    image_factory \u003d kwargs.get(\u0027image_factory\u0027)"},{"line_number":245,"context_line":"    uri \u003d kwargs.get(\u0027uri\u0027)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"    limbo \u003d lf.Flow(task_type)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    import_flows \u003d _get_import_flows(**kwargs)"},{"line_number":250,"context_line":"    if import_flows is not None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_9ca3a39d","line":247,"updated":"2015-02-23 14:57:58.000000000","message":"Why did you change it from uf to lf?\nFor me it makes sense to be uf as ImportToStore and import_flows can be run in parallel.","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"deefe1b0882de391ba804520d883f7212cae5e43","unresolved":false,"context_lines":[{"line_number":244,"context_line":"    image_factory \u003d kwargs.get(\u0027image_factory\u0027)"},{"line_number":245,"context_line":"    uri \u003d kwargs.get(\u0027uri\u0027)"},{"line_number":246,"context_line":""},{"line_number":247,"context_line":"    limbo \u003d lf.Flow(task_type)"},{"line_number":248,"context_line":""},{"line_number":249,"context_line":"    import_flows \u003d _get_import_flows(**kwargs)"},{"line_number":250,"context_line":"    if import_flows is not None:"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_09e922b8","line":247,"in_reply_to":"ba7be1f8_9ca3a39d","updated":"2015-02-23 23:39:14.000000000","message":"I\u0027ve a local change that improves this a bit more. The reason is that `ImportToStore` used to be a task in this flow and we can\u0027t run that task before we go through the imported tasks because it changes the image state to \u0027saving\u0027 preventing tasks like `Introspection` to change the value of certain fields (e.g: disk_format)","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"ef81195d213a0839a300fd524ad62ebb6ebabef3","unresolved":false,"context_lines":[{"line_number":252,"context_line":""},{"line_number":253,"context_line":"    flow \u003d lf.Flow(task_type).add("},{"line_number":254,"context_line":"        _CreateImage(task_id, task_type, task_repo, image_repo, image_factory),"},{"line_number":255,"context_line":"        limbo,"},{"line_number":256,"context_line":"        _ImportToStore(task_id, task_type, image_repo, uri),"},{"line_number":257,"context_line":"        _SaveImage(task_id, task_type, image_repo),"},{"line_number":258,"context_line":"        _CompleteTask(task_id, task_type, task_repo)"}],"source_content_type":"text/x-python","patch_set":8,"id":"ba7be1f8_af648449","line":255,"updated":"2015-02-24 11:22:11.000000000","message":"I just realized that if the limbo taskflow fails the image will not get imported. This is ok if we are converting the image, but wrong if we are doing an introspection. Right?","commit_id":"9c21e73501d728da5056b4e67b8b2f3b7025dd58"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"baa70e92f7b808f93a610bcd06b4d0506cc871da","unresolved":false,"context_lines":[{"line_number":214,"context_line":"            # necessary"},{"line_number":215,"context_line":"            err_msg \u003d (\"Error: \" + six.text_type(type(e)) + \u0027: \u0027 +"},{"line_number":216,"context_line":"                       common_utils.exception_to_str(e))"},{"line_number":217,"context_line":"            log_msg \u003d _LE(err_msg + (\"Task ID %s\" % task.task_id))  # noqa"},{"line_number":218,"context_line":"            LOG.exception(log_msg)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"            task.fail(_LE(err_msg))  # noqa"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_ed964c4b","line":217,"updated":"2015-02-25 13:42:45.000000000","message":"_LE not defined, so please bring i18i in when removing the # noqas\n\nstill the whitespace","commit_id":"e4e0d47f523663f2501879f5367c3fdaf8bd418b"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"f1c525127d5c29ce363c8b70a8ce31edb8b0ff72","unresolved":false,"context_lines":[{"line_number":214,"context_line":"            # necessary"},{"line_number":215,"context_line":"            err_msg \u003d (\"Error: \" + six.text_type(type(e)) + \u0027: \u0027 +"},{"line_number":216,"context_line":"                       common_utils.exception_to_str(e))"},{"line_number":217,"context_line":"            log_msg \u003d _LE(err_msg + (\"Task ID %s\" % task.task_id))  # noqa"},{"line_number":218,"context_line":"            LOG.exception(log_msg)"},{"line_number":219,"context_line":""},{"line_number":220,"context_line":"            task.fail(_LE(err_msg))  # noqa"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_a6f1d102","line":217,"in_reply_to":"ba7be1f8_ed964c4b","updated":"2015-02-25 14:07:23.000000000","message":"forgot, will do","commit_id":"e4e0d47f523663f2501879f5367c3fdaf8bd418b"},{"author":{"_account_id":8984,"name":"George Peristerakis","email":"gperiste@redhat.com","username":"peristeri"},"change_message_id":"5127154ea6da536d5853d6754bd1da8407d896d5","unresolved":false,"context_lines":[{"line_number":49,"context_line":"                               self.__class__.__name__[1:],"},{"line_number":50,"context_line":"                               task_id))"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def execute(self):"},{"line_number":53,"context_line":"        task \u003d script_utils.get_task(self.task_repo, self.task_id)"},{"line_number":54,"context_line":"        if task is None:"},{"line_number":55,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_94c86c56","line":52,"updated":"2015-02-25 16:50:15.000000000","message":"The logic look good. I have a request, for debugging purposes can you add log.info at the begin and end of each execute function? The reason I ask is, the functions are long running and the flow is dynamically created. It would be easier to trace (both staging and production) if there\u0027s a problem with the flow.","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"7e6c96d030d4ed530e5ed6c35213f027f5ceb98a","unresolved":false,"context_lines":[{"line_number":49,"context_line":"                               self.__class__.__name__[1:],"},{"line_number":50,"context_line":"                               task_id))"},{"line_number":51,"context_line":""},{"line_number":52,"context_line":"    def execute(self):"},{"line_number":53,"context_line":"        task \u003d script_utils.get_task(self.task_repo, self.task_id)"},{"line_number":54,"context_line":"        if task is None:"},{"line_number":55,"context_line":"            return"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_5ab37de8","line":52,"in_reply_to":"ba7be1f8_94c86c56","updated":"2015-02-25 17:08:56.000000000","message":"Hey, I don\u0027t think adding an info log twice for each execute is a good idea. That will end up being really verbose and not all operations are long running. For example, this one here shouldn\u0027t take long to end since it just creates the image in the database.\n\nProbably debug ? Even then, it may be too verbose.","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d9fce01df217bdb69ebb9da179af25a3d41a0a8e","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    # the duplicated calls to those tasks by creating a linear flow that"},{"line_number":278,"context_line":"    # ensures those are called before the other tasks."},{"line_number":279,"context_line":"    # For now, I\u0027m keeping them here, though."},{"line_number":280,"context_line":"    limbo \u003d lf.Flow(task_type).add(_ImportToFS(task_id,"},{"line_number":281,"context_line":"                                               task_type,"},{"line_number":282,"context_line":"                                               task_repo,"},{"line_number":283,"context_line":"                                               uri))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_20d7774c","line":280,"updated":"2015-02-25 18:58:03.000000000","message":"How do you create the just rollback on just subflows?\nI\u0027m missing something, because as far as I see it reverts on every failure and it\u0027s thanks to your OptionalTask that it won\u0027t revert if introspection fails.\nAnyhow, then you should not include import_to_store to limbo on L299 as that breaks the isolation you mention by including the task in the subflow, contrary to what happens on the case that there are no extensions (L311)","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"486b0d978663d60e9edab12d14130c8abceae1d6","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    # the duplicated calls to those tasks by creating a linear flow that"},{"line_number":278,"context_line":"    # ensures those are called before the other tasks."},{"line_number":279,"context_line":"    # For now, I\u0027m keeping them here, though."},{"line_number":280,"context_line":"    limbo \u003d lf.Flow(task_type).add(_ImportToFS(task_id,"},{"line_number":281,"context_line":"                                               task_type,"},{"line_number":282,"context_line":"                                               task_repo,"},{"line_number":283,"context_line":"                                               uri))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_0ebef6ad","line":280,"updated":"2015-02-26 17:22:30.000000000","message":"I think I follow the idea, but I have a question.\nWouldn\u0027t the image not get stored if the Import to FS fails (let\u0027s say due to lack of space)?\nAnd that isn\u0027t great, because the service is actually able to import it if it doesn\u0027t try to store it for pre-processing.\nAnd I\u0027m talking about introspection here, not conversion, where it makes sense to fail the import.","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d8cc4cd9942273da4c5a95062e9f7331f347b2b4","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    # the duplicated calls to those tasks by creating a linear flow that"},{"line_number":278,"context_line":"    # ensures those are called before the other tasks."},{"line_number":279,"context_line":"    # For now, I\u0027m keeping them here, though."},{"line_number":280,"context_line":"    limbo \u003d lf.Flow(task_type).add(_ImportToFS(task_id,"},{"line_number":281,"context_line":"                                               task_type,"},{"line_number":282,"context_line":"                                               task_repo,"},{"line_number":283,"context_line":"                                               uri))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_1a0bc58e","line":280,"updated":"2015-02-25 17:09:18.000000000","message":"I would not create a different flow (since both are linear) and would use a list to add the tasks until we finally create it.\nI believe that way it\u0027s a little more readable.\nSo it would go like:\nlimbo \u003d [_ImportToFS(task_id, task_type, task_repo, uri),\n         *_get_import_flows(**kwargs)]\n\nimport_to_store \u003d ...\n\nif (len(limbo) \u003e 1):\n    limbo.extend((import_to_store, _DeleteFromFS(task_id, task_type)]\nelse:\n    limbo \u003d [import_to_store]\n\nlimbo.extend(\n    _SaveImage(...)\n    _CompleteTask(...)\n)\n\nflow \u003d lf.Flow(.....).add(\n    _CreateImage(....),\n    *limbo)","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"01f52d193b41ce18bd2babdb76dbce0a57d48f19","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    # the duplicated calls to those tasks by creating a linear flow that"},{"line_number":278,"context_line":"    # ensures those are called before the other tasks."},{"line_number":279,"context_line":"    # For now, I\u0027m keeping them here, though."},{"line_number":280,"context_line":"    limbo \u003d lf.Flow(task_type).add(_ImportToFS(task_id,"},{"line_number":281,"context_line":"                                               task_type,"},{"line_number":282,"context_line":"                                               task_repo,"},{"line_number":283,"context_line":"                                               uri))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_c2f3c69d","line":280,"in_reply_to":"ba7be1f8_0ebef6ad","updated":"2015-02-26 21:50:12.000000000","message":"mmh, you do have a good point there.\n\nI think the other thing that made me put the task in the subflow was that the subflow was not providing correctly the file_path argument. But I think that was fixed.\n\nLets move it out for now, thanks.","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"d9d349eebe6922361b38e0790b30c9a45f8827e0","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    # the duplicated calls to those tasks by creating a linear flow that"},{"line_number":278,"context_line":"    # ensures those are called before the other tasks."},{"line_number":279,"context_line":"    # For now, I\u0027m keeping them here, though."},{"line_number":280,"context_line":"    limbo \u003d lf.Flow(task_type).add(_ImportToFS(task_id,"},{"line_number":281,"context_line":"                                               task_type,"},{"line_number":282,"context_line":"                                               task_repo,"},{"line_number":283,"context_line":"                                               uri))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_ca1f9cb5","line":280,"in_reply_to":"ba7be1f8_1a0bc58e","updated":"2015-02-25 18:23:01.000000000","message":"At first, I wanted to do that but I would prefer not to. The reason is that the subflow gives us extra isolation to control how the parent flow will be executed.\n\nIn L302, the parent flow is created with a revert logic that forces the rollback of *just* sub-flows and not the entire execution. If we merge both flows, we won\u0027t be able to do this.","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"7befe0966450809210ae20ab84bda8ad8f1f6b0c","unresolved":false,"context_lines":[{"line_number":277,"context_line":"    # the duplicated calls to those tasks by creating a linear flow that"},{"line_number":278,"context_line":"    # ensures those are called before the other tasks."},{"line_number":279,"context_line":"    # For now, I\u0027m keeping them here, though."},{"line_number":280,"context_line":"    limbo \u003d lf.Flow(task_type).add(_ImportToFS(task_id,"},{"line_number":281,"context_line":"                                               task_type,"},{"line_number":282,"context_line":"                                               task_repo,"},{"line_number":283,"context_line":"                                               uri))"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_dfad839c","line":280,"in_reply_to":"ba7be1f8_20d7774c","updated":"2015-02-26 07:43:52.000000000","message":"The `revert subflow` thing is done by adding the `retry\u003dretry.AlwaysRevert()` atom. It\u0027s true that in the case of the Introspection task it doesn\u0027t make much sense but it does for the Convert task.\n\nThe reason I\u0027m adding the `import_to_flow` to limbo is because in the 2 scenarios we currently have, it makes sense to run it as part of this flow but *after* the import tasks run. For example:\n\n- In the case Introspection runs, we\u0027d have downloaded the image locally and it\u0027d be good to take advantage of this fact. Even if the Introspection task fails, it\u0027s fine to run the ImportToStore task. If either ImportToFS or DeleteFromFS fail, it makes sense to revert the subflow. I\u0027m not sure about the later, though.\n\n- In the case of Convert, there\u0027s no point in running ImportToFlow if the Convert task fails because we need to import the converted image and not the one provided by the user.\n\nIn the case of ImportToStore running in a subflow and failing, I believe it\u0027s fine to not rollback the whole parent flow. The reason being the user can still upload an image manually if they want. It\u0027ll be the users call what to do with that image.\n\nOne more note, this subflow also protects glance from the user adding custom entry points and failing. Although, if the user does that, it\u0027s their problem.\n\nHonestly, I don\u0027t think the subflow is expensive so I wouldn\u0027t mind having a couple of more subflows just to create a logical structure for this execution.\n\nThat said, I believe this will change a bit now after adding the configuration variable.","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"44e6afc37fa80fe0607e17e0b0e112fa42afad58","unresolved":false,"context_lines":[{"line_number":408,"context_line":"        # make sure ImportToStore is called *after* the imported"},{"line_number":409,"context_line":"        # flow stages. If not, the image will be set to saving state"},{"line_number":410,"context_line":"        # invalidating tasks like Introspection or Convert."},{"line_number":411,"context_line":"        _ImportToStore(task_id, task_type, image_repo, uri),"},{"line_number":412,"context_line":"        _SaveImage(task_id, task_type, image_repo),"},{"line_number":413,"context_line":"        _CompleteTask(task_id, task_type, task_repo)"},{"line_number":414,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":15,"id":"ba7be1f8_c443c16a","line":411,"updated":"2015-02-27 08:55:57.000000000","message":"I haven\u0027t worked with subflows so I\u0027m not sure if what I\u0027m saying makes sense.\nWouldn\u0027t it be a problem if?:\n - ImportToFS creates the file and provides file_path to flow\n - DeleteFromFS remove file but doesn\u0027t alter file_path\n - ImportToStore receives file_path argument with data and tries to load from it\nI don\u0027t know if generated data from subflow is automatically available to parent flow. If it\u0027s not then it\u0027s fine, but if it is available then ImportToStore would receive an invalid file_path.","commit_id":"e5d7a682dd1de085f48630fcb805b1407296c9b6"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"0b641b6a5fca3e0e772ac09b991de86a9116f935","unresolved":false,"context_lines":[{"line_number":408,"context_line":"        # make sure ImportToStore is called *after* the imported"},{"line_number":409,"context_line":"        # flow stages. If not, the image will be set to saving state"},{"line_number":410,"context_line":"        # invalidating tasks like Introspection or Convert."},{"line_number":411,"context_line":"        _ImportToStore(task_id, task_type, image_repo, uri),"},{"line_number":412,"context_line":"        _SaveImage(task_id, task_type, image_repo),"},{"line_number":413,"context_line":"        _CompleteTask(task_id, task_type, task_repo)"},{"line_number":414,"context_line":"    )"}],"source_content_type":"text/x-python","patch_set":15,"id":"ba7be1f8_6d059113","line":411,"in_reply_to":"ba7be1f8_c443c16a","updated":"2015-02-27 12:27:46.000000000","message":"And this was the other reason why I added ImportToStore to the subflow and I had forgotten about it. (facepalm)\n\nFWIW, I think the previous version was better than this one. What I\u0027m going to do is add the ImportToStore twice and a mechanism to understand whether the image data has already been imported into the store.","commit_id":"e5d7a682dd1de085f48630fcb805b1407296c9b6"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":70,"context_line":"        return image.image_id"},{"line_number":71,"context_line":""},{"line_number":72,"context_line":"    def revert(self, *args, **kwargs):"},{"line_number":73,"context_line":"        # NOTE(flaper87): Define the revert rules for images on failures."},{"line_number":74,"context_line":"        # Deleting the image may not be what we want since users could upload"},{"line_number":75,"context_line":"        # the image data in a separate step. However, it really depends on"},{"line_number":76,"context_line":"        # when the failure happened. I guess we should check if data has been"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_e0c9ea26","line":73,"updated":"2015-03-03 23:34:28.000000000","message":"NOTE or TODO?","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":90,"context_line":"        self.uri \u003d uri"},{"line_number":91,"context_line":"        super(_ImportToFS, self).__init__("},{"line_number":92,"context_line":"            name\u003d\u0027%s-%s-%s\u0027 % (task_type,"},{"line_number":93,"context_line":"                               self.__class__.__name__[1:],"},{"line_number":94,"context_line":"                               task_id))"},{"line_number":95,"context_line":""},{"line_number":96,"context_line":"        if CONF.task.work_dir is None:"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_60041a4a","line":93,"updated":"2015-03-03 23:34:28.000000000","message":"Can I ask that we explain the reason for the slice? I understand it\u0027s to exclude the leading _ but it\u0027d be nice to have that noted so that it can be changed if the class name changes or we change how name is constructed.","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        self.store \u003d self._build_store()"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def _build_store(self):"},{"line_number":107,"context_line":"        # NOTE(flaper87): Due to the nice glance_store api (#sarcasm), we\u0027re"},{"line_number":108,"context_line":"        # forced to build our own config object, register the required options"},{"line_number":109,"context_line":"        # (and by requierd I mean *ALL* of them, even the ones we don\u0027t want),"},{"line_number":110,"context_line":"        # and create our own store instance by calling a private function."}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_40b8f674","line":107,"updated":"2015-03-03 23:34:28.000000000","message":"Comments aren\u0027t really the best place for sarcasm.","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":104,"context_line":"        self.store \u003d self._build_store()"},{"line_number":105,"context_line":""},{"line_number":106,"context_line":"    def _build_store(self):"},{"line_number":107,"context_line":"        # NOTE(flaper87): Due to the nice glance_store api (#sarcasm), we\u0027re"},{"line_number":108,"context_line":"        # forced to build our own config object, register the required options"},{"line_number":109,"context_line":"        # (and by requierd I mean *ALL* of them, even the ones we don\u0027t want),"},{"line_number":110,"context_line":"        # and create our own store instance by calling a private function."}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_9909392f","line":107,"in_reply_to":"ba7be1f8_40b8f674","updated":"2015-03-04 13:52:36.000000000","message":"oh come on, I\u0027m sorry but this is way to nitpicky, imho. Lets not kill the fun in what we\u0027re doing here.","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":106,"context_line":"    def _build_store(self):"},{"line_number":107,"context_line":"        # NOTE(flaper87): Due to the nice glance_store api (#sarcasm), we\u0027re"},{"line_number":108,"context_line":"        # forced to build our own config object, register the required options"},{"line_number":109,"context_line":"        # (and by requierd I mean *ALL* of them, even the ones we don\u0027t want),"},{"line_number":110,"context_line":"        # and create our own store instance by calling a private function."},{"line_number":111,"context_line":"        # This is certainly unfortunate but it\u0027s the best we can do until the"},{"line_number":112,"context_line":"        # glance_store refactor is done. A good thing is that glance_store is"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_a091b2f2","line":109,"updated":"2015-03-03 23:34:28.000000000","message":"required","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        # This is certainly unfortunate but it\u0027s the best we can do until the"},{"line_number":112,"context_line":"        # glance_store refactor is done. A good thing is that glance_store is"},{"line_number":113,"context_line":"        # under our team\u0027s management and it gates on Glance so changes to"},{"line_number":114,"context_line":"        # this API will (should?) break task\u0027s tests."},{"line_number":115,"context_line":"        conf \u003d cfg.ConfigOpts()"},{"line_number":116,"context_line":"        backend.register_opts(conf)"},{"line_number":117,"context_line":"        conf.set_override(\u0027filesystem_store_datadir\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_60609a01","line":114,"updated":"2015-03-03 23:34:28.000000000","message":"Which is also a bad thing because when those changes are made, it\u0027ll be dependent on a patch in Glance to  unbreak its gate, but that patch will only pass if the glance_store change is merged (so there will be a cyclic dependency).","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":111,"context_line":"        # This is certainly unfortunate but it\u0027s the best we can do until the"},{"line_number":112,"context_line":"        # glance_store refactor is done. A good thing is that glance_store is"},{"line_number":113,"context_line":"        # under our team\u0027s management and it gates on Glance so changes to"},{"line_number":114,"context_line":"        # this API will (should?) break task\u0027s tests."},{"line_number":115,"context_line":"        conf \u003d cfg.ConfigOpts()"},{"line_number":116,"context_line":"        backend.register_opts(conf)"},{"line_number":117,"context_line":"        conf.set_override(\u0027filesystem_store_datadir\u0027,"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_99f3f909","line":114,"in_reply_to":"ba7be1f8_60609a01","updated":"2015-03-04 13:52:36.000000000","message":"The comment was more like: \"If we propose a patch that breaks, we\u0027ll see it before it lands\" That\u0027s why we\u0027re gating this things.","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        if result is None:"},{"line_number":162,"context_line":"            return"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if os.path.exists(result.split(\"file://\")[-1]):"},{"line_number":165,"context_line":"            store_api.delete_from_backend(result)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_833e78b6","line":164,"updated":"2015-03-03 23:34:28.000000000","message":"Is result here a URL? Do I understand correctly that result will be\n\n    file:///path/to/file\n\nIf so, why not do something like the following\n\n    from six.moves.urllib import parse as urlparse\n\n    parsed_result \u003d urlparse(result)\n    if (parsed_result.scheme \u003d\u003d \u0027file://\u0027 and\n            parsed_result.path and\n            os.path.exists(parsed_result.path)):\n        store_api.delete_from_backend(result)","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":161,"context_line":"        if result is None:"},{"line_number":162,"context_line":"            return"},{"line_number":163,"context_line":""},{"line_number":164,"context_line":"        if os.path.exists(result.split(\"file://\")[-1]):"},{"line_number":165,"context_line":"            store_api.delete_from_backend(result)"},{"line_number":166,"context_line":""},{"line_number":167,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_f91cf515","line":164,"in_reply_to":"ba7be1f8_833e78b6","updated":"2015-03-04 13:52:36.000000000","message":"`result` is an URL yes. It\u0027s an url coming from the `_ImportToFS` task.\n\nAlthough your proposed code is fine, I believe it\u0027s way more complicated than it should. We don\u0027t really care about the URL form here since it\u0027s supposed to come from the previous task. If the URL doesn\u0027t respect that form, then the `os.path.exists` will return False.\n\nI don\u0027t think we need to import urlparse and parse the URL, then check if there\u0027s a path and then check if the file exists. The code in this task is not critical and if the URL doesn\u0027t respect that form, it\u0027ll just move on.\n\nThe code you proposed does the exact same thing with more checks than are needed here.","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def execute(self, image_id, file_path\u003dNone):"},{"line_number":199,"context_line":"        # NOTE(flaper87): There are a couple of interesting bits in the"},{"line_number":200,"context_line":"        # interaccion between this task and the `_ImportToFS` one. I\u0027ll try"},{"line_number":201,"context_line":"        # to cover them in this comment."},{"line_number":202,"context_line":"        #"},{"line_number":203,"context_line":"        # NOTE:"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_ef0715c8","line":200,"updated":"2015-03-03 23:34:28.000000000","message":"interaction? I prefer the Italian form too, but it seems out of place in a body of english. Perhaps write the entire block comment in Italian?","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":197,"context_line":""},{"line_number":198,"context_line":"    def execute(self, image_id, file_path\u003dNone):"},{"line_number":199,"context_line":"        # NOTE(flaper87): There are a couple of interesting bits in the"},{"line_number":200,"context_line":"        # interaccion between this task and the `_ImportToFS` one. I\u0027ll try"},{"line_number":201,"context_line":"        # to cover them in this comment."},{"line_number":202,"context_line":"        #"},{"line_number":203,"context_line":"        # NOTE:"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_39176def","line":200,"in_reply_to":"ba7be1f8_ef0715c8","updated":"2015-03-04 13:52:36.000000000","message":"LOOOOL :D fixing","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        # conversion enabled"},{"line_number":228,"context_line":"        #"},{"line_number":229,"context_line":"        # In this scenario, the user would benefit from having a *local* store"},{"line_number":230,"context_line":"        # into which the image can be converted. This will required downloading"},{"line_number":231,"context_line":"        # the image locally, converting it and then copying the converted image"},{"line_number":232,"context_line":"        # to the remote store."},{"line_number":233,"context_line":"        #"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_ef2e3542","line":230,"updated":"2015-03-03 23:34:28.000000000","message":"will require","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":227,"context_line":"        # conversion enabled"},{"line_number":228,"context_line":"        #"},{"line_number":229,"context_line":"        # In this scenario, the user would benefit from having a *local* store"},{"line_number":230,"context_line":"        # into which the image can be converted. This will required downloading"},{"line_number":231,"context_line":"        # the image locally, converting it and then copying the converted image"},{"line_number":232,"context_line":"        # to the remote store."},{"line_number":233,"context_line":"        #"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_d9e65116","line":230,"in_reply_to":"ba7be1f8_ef2e3542","updated":"2015-03-04 13:52:36.000000000","message":"Done","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":236,"context_line":"        # Scenario 4: FS Store is local, introspection enabled,"},{"line_number":237,"context_line":"        # conversion enabled"},{"line_number":238,"context_line":"        #"},{"line_number":239,"context_line":"        # In both this scenarios the user shouldn\u0027t care, therefore probably"},{"line_number":240,"context_line":"        # benefit, about the scratch path and the FS store being the same from"},{"line_number":241,"context_line":"        # a performance perspective. Space wise, regardless of the scenario,"},{"line_number":242,"context_line":"        # the user will have to account for it in advance."}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_efd5551c","line":239,"updated":"2015-03-03 23:34:28.000000000","message":"This sentence is a little confusing. Could you update it please?","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":236,"context_line":"        # Scenario 4: FS Store is local, introspection enabled,"},{"line_number":237,"context_line":"        # conversion enabled"},{"line_number":238,"context_line":"        #"},{"line_number":239,"context_line":"        # In both this scenarios the user shouldn\u0027t care, therefore probably"},{"line_number":240,"context_line":"        # benefit, about the scratch path and the FS store being the same from"},{"line_number":241,"context_line":"        # a performance perspective. Space wise, regardless of the scenario,"},{"line_number":242,"context_line":"        # the user will have to account for it in advance."}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_d9949146","line":239,"in_reply_to":"ba7be1f8_efd5551c","updated":"2015-03-04 13:52:36.000000000","message":"Done","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":265,"context_line":"        #    image_import.set_image_data(image, self.uri, None)"},{"line_number":266,"context_line":"        #    return"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        # NOTE(flaper87): Don\u0027t assume the image was store in the"},{"line_number":269,"context_line":"        # work_dir. Think in the case this path was provided by another task."},{"line_number":270,"context_line":"        # Also, lets try not to neither assume things nor create \"logic\""},{"line_number":271,"context_line":"        # dependencies between this task and `_ImportToFS`"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_0fc0e94e","line":268,"updated":"2015-03-03 23:34:28.000000000","message":"was stored.","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":265,"context_line":"        #    image_import.set_image_data(image, self.uri, None)"},{"line_number":266,"context_line":"        #    return"},{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        # NOTE(flaper87): Don\u0027t assume the image was store in the"},{"line_number":269,"context_line":"        # work_dir. Think in the case this path was provided by another task."},{"line_number":270,"context_line":"        # Also, lets try not to neither assume things nor create \"logic\""},{"line_number":271,"context_line":"        # dependencies between this task and `_ImportToFS`"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_399ccd69","line":268,"in_reply_to":"ba7be1f8_0fc0e94e","updated":"2015-03-04 13:52:36.000000000","message":"Done","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        # NOTE(flaper87): Don\u0027t assume the image was store in the"},{"line_number":269,"context_line":"        # work_dir. Think in the case this path was provided by another task."},{"line_number":270,"context_line":"        # Also, lets try not to neither assume things nor create \"logic\""},{"line_number":271,"context_line":"        # dependencies between this task and `_ImportToFS`"},{"line_number":272,"context_line":"        #"},{"line_number":273,"context_line":"        # base_path \u003d os.path.dirname(file_path.split(\"file://\")[-1])"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_cfc1714f","line":270,"updated":"2015-03-03 23:34:28.000000000","message":"let\u0027s try to neither assume things nor create","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":267,"context_line":""},{"line_number":268,"context_line":"        # NOTE(flaper87): Don\u0027t assume the image was store in the"},{"line_number":269,"context_line":"        # work_dir. Think in the case this path was provided by another task."},{"line_number":270,"context_line":"        # Also, lets try not to neither assume things nor create \"logic\""},{"line_number":271,"context_line":"        # dependencies between this task and `_ImportToFS`"},{"line_number":272,"context_line":"        #"},{"line_number":273,"context_line":"        # base_path \u003d os.path.dirname(file_path.split(\"file://\")[-1])"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_b987dd0e","line":270,"in_reply_to":"ba7be1f8_cfc1714f","updated":"2015-03-04 13:52:36.000000000","message":"Done","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":307,"context_line":"        if new_image.status \u003d\u003d \u0027saving\u0027:"},{"line_number":308,"context_line":"            # NOTE(flaper87): THIS IS WRONG!"},{"line_number":309,"context_line":"            # we should be doing atomic updates to avoid"},{"line_number":310,"context_line":"            # race conditions. This happens in other places"},{"line_number":311,"context_line":"            # too."},{"line_number":312,"context_line":"            new_image.status \u003d \u0027active\u0027"},{"line_number":313,"context_line":"        self.image_repo.save(new_image)"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_af9d1d68","line":310,"updated":"2015-03-03 23:34:28.000000000","message":"\u003d(","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"0265456de849f8b9a7719a78e5f23413f58d4d64","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        # With the glance_store refactor, this code will change, until"},{"line_number":120,"context_line":"        # that happens, we don\u0027t have a better option and this is the"},{"line_number":121,"context_line":"        # least worst one, IMHO."},{"line_number":122,"context_line":"        store \u003d backend._load_store(conf, \u0027file\u0027)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        if store is None:"},{"line_number":125,"context_line":"            msg \u003d (_(\"%(task_id)s of %(task_type)s not configured \""}],"source_content_type":"text/x-python","patch_set":23,"id":"9a80dd14_eaa8ac21","line":122,"updated":"2015-03-10 13:09:29.000000000","message":":(","commit_id":"0d7c1a4722ab87528855c9a9ca74d2dbb04887e1"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"77ec8e32ae08fba2ffda7ae702a4eea538675cd2","unresolved":false,"context_lines":[{"line_number":119,"context_line":"        # With the glance_store refactor, this code will change, until"},{"line_number":120,"context_line":"        # that happens, we don\u0027t have a better option and this is the"},{"line_number":121,"context_line":"        # least worst one, IMHO."},{"line_number":122,"context_line":"        store \u003d backend._load_store(conf, \u0027file\u0027)"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"        if store is None:"},{"line_number":125,"context_line":"            msg \u003d (_(\"%(task_id)s of %(task_type)s not configured \""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a80dd14_3bf4d459","line":122,"updated":"2015-03-10 14:00:30.000000000","message":"This may break with the glance_store patch to allow BadStoreConfiguration through to users.","commit_id":"0503fdc2d81097d9453ce6ad844785fbb7905b79"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"77ec8e32ae08fba2ffda7ae702a4eea538675cd2","unresolved":false,"context_lines":[{"line_number":186,"context_line":"        self.image_repo \u003d image_repo"},{"line_number":187,"context_line":"        self.uri \u003d uri"},{"line_number":188,"context_line":"        super(_ImportToStore, self).__init__("},{"line_number":189,"context_line":"            name\u003d\u0027%s-%s-%s\u0027 % (task_type,"},{"line_number":190,"context_line":"                               self.__class__.__name__[1:],"},{"line_number":191,"context_line":"                               task_id))"},{"line_number":192,"context_line":""}],"source_content_type":"text/x-python","patch_set":24,"id":"9a80dd14_2703da0e","line":189,"updated":"2015-03-10 14:00:30.000000000","message":"You changed the other tasks\u0027 init methods to use an explicit class-name string. Why not here?","commit_id":"0503fdc2d81097d9453ce6ad844785fbb7905b79"}],"glance/async/flows/introspect.py":[{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"60676e5901940d0182fbd6bc3633fa8214d78d64","unresolved":false,"context_lines":[{"line_number":57,"context_line":"def get_flow(**kwargs):"},{"line_number":58,"context_line":"    task_id \u003d kwargs.get(\u0027task_id\u0027)"},{"line_number":59,"context_line":"    task_type \u003d kwargs.get(\u0027task_type\u0027)"},{"line_number":60,"context_line":"    task_repo \u003d kwargs.get(\u0027task_repo\u0027)"},{"line_number":61,"context_line":"    image_repo \u003d kwargs.get(\u0027image_repo\u0027)"},{"line_number":62,"context_line":"    image_factory \u003d kwargs.get(\u0027image_factory\u0027)"},{"line_number":63,"context_line":"    uri \u003d kwargs.get(\u0027uri\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da86d52c_a8490534","line":60,"updated":"2015-02-16 17:10:58.000000000","message":"It seems we don\u0027t use task_repo, image_factory, or uri here.\n\nProbably copy-paste from base_import.py","commit_id":"15bf927650fdf77d68d82aea0bee7ed363c2fd9a"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"46b3151117c73c895f02f4c45aef328530ea4449","unresolved":false,"context_lines":[{"line_number":57,"context_line":"def get_flow(**kwargs):"},{"line_number":58,"context_line":"    task_id \u003d kwargs.get(\u0027task_id\u0027)"},{"line_number":59,"context_line":"    task_type \u003d kwargs.get(\u0027task_type\u0027)"},{"line_number":60,"context_line":"    task_repo \u003d kwargs.get(\u0027task_repo\u0027)"},{"line_number":61,"context_line":"    image_repo \u003d kwargs.get(\u0027image_repo\u0027)"},{"line_number":62,"context_line":"    image_factory \u003d kwargs.get(\u0027image_factory\u0027)"},{"line_number":63,"context_line":"    uri \u003d kwargs.get(\u0027uri\u0027)"}],"source_content_type":"text/x-python","patch_set":1,"id":"da86d52c_bb8e9525","line":60,"in_reply_to":"da86d52c_a8490534","updated":"2015-02-17 12:59:12.000000000","message":"Done","commit_id":"15bf927650fdf77d68d82aea0bee7ed363c2fd9a"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a6015f19d8bb8e781e8dc147787a3d8a899f5302","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        metadata \u003d json.loads(stdout)"},{"line_number":58,"context_line":"        image_id \u003d image.image_id"},{"line_number":59,"context_line":"        new_image \u003d self.image_repo.get(image_id)"},{"line_number":60,"context_line":"        new_image.virtual_size \u003d metadata.get(\u0027virtual-size\u0027, 0)"},{"line_number":61,"context_line":"        self.image_repo.save(new_image)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_a5969292","line":60,"updated":"2015-02-23 12:14:50.000000000","message":"Why did you remove `new_image.disk_format \u003d metadata.get(\u0027format\u0027)`?\nWe could set it if quemu-img gives that info, and if it doesn\u0027t we could use \u0027import_from_format\u0027 task argument if it has a proper value or leave it as is if it doesn\u0027t.","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"01e8f2c1be2e2dad64cb73af1f4910907309bf9f","unresolved":false,"context_lines":[{"line_number":57,"context_line":"        metadata \u003d json.loads(stdout)"},{"line_number":58,"context_line":"        image_id \u003d image.image_id"},{"line_number":59,"context_line":"        new_image \u003d self.image_repo.get(image_id)"},{"line_number":60,"context_line":"        new_image.virtual_size \u003d metadata.get(\u0027virtual-size\u0027, 0)"},{"line_number":61,"context_line":"        self.image_repo.save(new_image)"},{"line_number":62,"context_line":""},{"line_number":63,"context_line":""}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_1b5f9642","line":60,"in_reply_to":"ba7be1f8_a5969292","updated":"2015-02-24 10:01:46.000000000","message":"The reason is that there was a conflict with the ImportToStore task, which changes the image status to `saving` and makes it impossible to change some attributes like the `disk_format`. I changed this in the last PS.","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        self.image_repo \u003d image_repo"},{"line_number":39,"context_line":"        super(_Introspect, self).__init__("},{"line_number":40,"context_line":"            name\u003d\u0027%s-%s-%s\u0027 % (task_type,"},{"line_number":41,"context_line":"                               self.__class__.__name__[1:],"},{"line_number":42,"context_line":"                               task_id))"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def execute(self, image_id, file_path):"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_8fce9958","line":41,"updated":"2015-03-03 23:34:28.000000000","message":"Again we have this invocation that is being copied into several places. Is there a utility we can add to generate the name from these three things?\n\nAlso, would it make sense to call `.lstrip(\u0027_\u0027)` in the utility instead of slicing?","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":38,"context_line":"        self.image_repo \u003d image_repo"},{"line_number":39,"context_line":"        super(_Introspect, self).__init__("},{"line_number":40,"context_line":"            name\u003d\u0027%s-%s-%s\u0027 % (task_type,"},{"line_number":41,"context_line":"                               self.__class__.__name__[1:],"},{"line_number":42,"context_line":"                               task_id))"},{"line_number":43,"context_line":""},{"line_number":44,"context_line":"    def execute(self, image_id, file_path):"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_d93e5129","line":41,"in_reply_to":"ba7be1f8_8fce9958","updated":"2015-03-04 13:52:36.000000000","message":"Done","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"}],"glance/async/taskflow_executor.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a6015f19d8bb8e781e8dc147787a3d8a899f5302","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            \u0027engine\u0027: CONF.taskflow_executor.engine_mode,"},{"line_number":60,"context_line":"        }"},{"line_number":61,"context_line":"        self.engine_kwargs \u003d {}"},{"line_number":62,"context_line":"        if CONF.taskflow_executor.engine_mode \u003d\u003d \u0027parallel\u0027:"},{"line_number":63,"context_line":"            self.engine_kwargs[\u0027max_workers\u0027] \u003d ("},{"line_number":64,"context_line":"                CONF.taskflow_executor.max_workers)"},{"line_number":65,"context_line":"        super(TaskExecutor, self).__init__(context, task_repo, image_repo,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_65f49af2","line":62,"updated":"2015-02-23 12:14:50.000000000","message":"Should it be?\n`if CONF.taskflow_executor.engine_mode in (\u0027parallel\u0027, \u0027worker-based\u0027):`","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"5206cc6549851dd59c55e0395ce95398acb9107a","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            \u0027engine\u0027: CONF.taskflow_executor.engine_mode,"},{"line_number":60,"context_line":"        }"},{"line_number":61,"context_line":"        self.engine_kwargs \u003d {}"},{"line_number":62,"context_line":"        if CONF.taskflow_executor.engine_mode \u003d\u003d \u0027parallel\u0027:"},{"line_number":63,"context_line":"            self.engine_kwargs[\u0027max_workers\u0027] \u003d ("},{"line_number":64,"context_line":"                CONF.taskflow_executor.max_workers)"},{"line_number":65,"context_line":"        super(TaskExecutor, self).__init__(context, task_repo, image_repo,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_aaed5227","line":62,"updated":"2015-02-24 11:32:41.000000000","message":"You are right, I didn\u0027t see that the worker-based engine goes to a proxy.","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"01e8f2c1be2e2dad64cb73af1f4910907309bf9f","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            \u0027engine\u0027: CONF.taskflow_executor.engine_mode,"},{"line_number":60,"context_line":"        }"},{"line_number":61,"context_line":"        self.engine_kwargs \u003d {}"},{"line_number":62,"context_line":"        if CONF.taskflow_executor.engine_mode \u003d\u003d \u0027parallel\u0027:"},{"line_number":63,"context_line":"            self.engine_kwargs[\u0027max_workers\u0027] \u003d ("},{"line_number":64,"context_line":"                CONF.taskflow_executor.max_workers)"},{"line_number":65,"context_line":"        super(TaskExecutor, self).__init__(context, task_repo, image_repo,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_db0dfe91","line":62,"in_reply_to":"ba7be1f8_65f49af2","updated":"2015-02-24 10:01:46.000000000","message":"yes, it should. Good catch!","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"a33b8fd5548e8a4c5424b00b5c96f9eb4710ab71","unresolved":false,"context_lines":[{"line_number":59,"context_line":"            \u0027engine\u0027: CONF.taskflow_executor.engine_mode,"},{"line_number":60,"context_line":"        }"},{"line_number":61,"context_line":"        self.engine_kwargs \u003d {}"},{"line_number":62,"context_line":"        if CONF.taskflow_executor.engine_mode \u003d\u003d \u0027parallel\u0027:"},{"line_number":63,"context_line":"            self.engine_kwargs[\u0027max_workers\u0027] \u003d ("},{"line_number":64,"context_line":"                CONF.taskflow_executor.max_workers)"},{"line_number":65,"context_line":"        super(TaskExecutor, self).__init__(context, task_repo, image_repo,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_5ee65cb5","line":62,"in_reply_to":"ba7be1f8_db0dfe91","updated":"2015-02-24 10:18:28.000000000","message":"actually, no. This is just for the parallel engine. The number of workers in a worker-based deployment depends on how many workers you want to deploy.","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"a6015f19d8bb8e781e8dc147787a3d8a899f5302","unresolved":false,"context_lines":[{"line_number":72,"context_line":"                task_input.get(\u0027import_from\u0027))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"            kwds \u003d {"},{"line_number":75,"context_line":"                \u0027uri\u0027: uri,"},{"line_number":76,"context_line":"                \u0027task_id\u0027: task.task_id,"},{"line_number":77,"context_line":"                \u0027task_type\u0027: task.type,"},{"line_number":78,"context_line":"                \u0027context\u0027: self.context,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_e5e7aa3e","line":75,"updated":"2015-02-23 12:14:50.000000000","message":"Why don\u0027t we pass `**task_imput` so that tasks from workflow have this data and don\u0027t need to unpack it again?","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"01e8f2c1be2e2dad64cb73af1f4910907309bf9f","unresolved":false,"context_lines":[{"line_number":72,"context_line":"                task_input.get(\u0027import_from\u0027))"},{"line_number":73,"context_line":""},{"line_number":74,"context_line":"            kwds \u003d {"},{"line_number":75,"context_line":"                \u0027uri\u0027: uri,"},{"line_number":76,"context_line":"                \u0027task_id\u0027: task.task_id,"},{"line_number":77,"context_line":"                \u0027task_type\u0027: task.type,"},{"line_number":78,"context_line":"                \u0027context\u0027: self.context,"}],"source_content_type":"text/x-python","patch_set":6,"id":"ba7be1f8_3bd7fac0","line":75,"in_reply_to":"ba7be1f8_e5e7aa3e","updated":"2015-02-24 10:01:46.000000000","message":"The reasoning is that the task might change from the moment the entry_point is loaded and the task is unpacked. It\u0027s better to load the task state when needed instead of passing it over and work on a stalled state.\n\nThe question now is: Do we expect task inputs to change? mmh, I\u0027d probably say no.","commit_id":"31793ea282c1dd138e05c37cd6c1ba55955d2ac0"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"baa70e92f7b808f93a610bcd06b4d0506cc871da","unresolved":false,"context_lines":[{"line_number":43,"context_line":"               help\u003d_(\"The mode in which the engine will run. \""},{"line_number":44,"context_line":"                      \"Can be \u0027serial\u0027 or \u0027parallel\u0027.\")),"},{"line_number":45,"context_line":"    cfg.IntOpt(\u0027max_workers\u0027,"},{"line_number":46,"context_line":"               default\u003d50,"},{"line_number":47,"context_line":"               help\u003d_(\"The number of parallel activities executed at the \""},{"line_number":48,"context_line":"                      \"same time by the engine. The value can be greater \""},{"line_number":49,"context_line":"                      \"than one when the engine mode is \u0027parallel\u0027.\"),"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_3056f72a","line":46,"updated":"2015-02-25 13:42:45.000000000","message":"Just SOfS (Shaken Out of Sleeve) value or is there some intelligence behind this?","commit_id":"e4e0d47f523663f2501879f5367c3fdaf8bd418b"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"f1c525127d5c29ce363c8b70a8ce31edb8b0ff72","unresolved":false,"context_lines":[{"line_number":43,"context_line":"               help\u003d_(\"The mode in which the engine will run. \""},{"line_number":44,"context_line":"                      \"Can be \u0027serial\u0027 or \u0027parallel\u0027.\")),"},{"line_number":45,"context_line":"    cfg.IntOpt(\u0027max_workers\u0027,"},{"line_number":46,"context_line":"               default\u003d50,"},{"line_number":47,"context_line":"               help\u003d_(\"The number of parallel activities executed at the \""},{"line_number":48,"context_line":"                      \"same time by the engine. The value can be greater \""},{"line_number":49,"context_line":"                      \"than one when the engine mode is \u0027parallel\u0027.\"),"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_c67085de","line":46,"in_reply_to":"ba7be1f8_3056f72a","updated":"2015-02-25 14:07:23.000000000","message":"Both.\n\nWe used to have 2 (btw, this value was changed in the previous patch) but since we may either use pure python Threads or GreenThreads, I preferred to bump it up a bit without setting it to 1000 as it was set for our old eventlet_executor.\n\nIn theory, if we always have threads monkey-patched (which we currently do, AFAIR) we should end up using greenthreads every time. Using 50 to have some extra concurrency but still paying it safe.","commit_id":"e4e0d47f523663f2501879f5367c3fdaf8bd418b"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":113,"context_line":"        task \u003d script_utils.get_task(self.task_repo, task_id)"},{"line_number":114,"context_line":"        if task is None:"},{"line_number":115,"context_line":"            # NOTE: This happens if task is not found in the database. In"},{"line_number":116,"context_line":"            # such cases, there is no way to update the task status so,"},{"line_number":117,"context_line":"            # it\u0027s ignored here."},{"line_number":118,"context_line":"            return"},{"line_number":119,"context_line":""}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_ef2d55ff","line":116,"updated":"2015-03-03 23:34:28.000000000","message":"Thank you for the comment.","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"}],"glance/async/utils.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d8cc4cd9942273da4c5a95062e9f7331f347b2b4","unresolved":false,"context_lines":[{"line_number":29,"context_line":"        super(OptionalTask, self).__init__(*args, **kwargs)"},{"line_number":30,"context_line":"        self.execute \u003d self.__catch_all(self.execute)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def __catch_all(self, func):"},{"line_number":33,"context_line":"        # NOTE(flaper87): Read this comment before calling the MI6"},{"line_number":34,"context_line":"        # Here\u0027s the thing, there\u0027s no nice way to define \"optional\""},{"line_number":35,"context_line":"        # tasks. That is, tasks whose failure shouldn\u0027t affect the execution"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_fa07d17f","line":32,"updated":"2015-02-25 17:09:18.000000000","message":"Nice workaround!  XD","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"040790e420c70291f3d9241e2b89c3391285a9f9","unresolved":false,"context_lines":[{"line_number":29,"context_line":"        super(OptionalTask, self).__init__(*args, **kwargs)"},{"line_number":30,"context_line":"        self.execute \u003d self.__catch_all(self.execute)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def __catch_all(self, func):"},{"line_number":33,"context_line":"        # NOTE(flaper87): Read this comment before calling the MI6"},{"line_number":34,"context_line":"        # Here\u0027s the thing, there\u0027s no nice way to define \"optional\""},{"line_number":35,"context_line":"        # tasks. That is, tasks whose failure shouldn\u0027t affect the execution"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_daf68d5c","line":32,"updated":"2015-02-25 17:25:35.000000000","message":"Nit: Why use name mangling here? Is there a reason to make it harder to override __catch_all? (_catch_all may be preferential)","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"d9d349eebe6922361b38e0790b30c9a45f8827e0","unresolved":false,"context_lines":[{"line_number":29,"context_line":"        super(OptionalTask, self).__init__(*args, **kwargs)"},{"line_number":30,"context_line":"        self.execute \u003d self.__catch_all(self.execute)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def __catch_all(self, func):"},{"line_number":33,"context_line":"        # NOTE(flaper87): Read this comment before calling the MI6"},{"line_number":34,"context_line":"        # Here\u0027s the thing, there\u0027s no nice way to define \"optional\""},{"line_number":35,"context_line":"        # tasks. That is, tasks whose failure shouldn\u0027t affect the execution"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_ead540e5","line":32,"in_reply_to":"ba7be1f8_daf68d5c","updated":"2015-02-25 18:23:01.000000000","message":"@louis\n\n`_catch_all` \u003c- private\n\n\n`__catch_all` \u003c- if you dare to hack it, I\u0027ll chase you down and 3 generations above/below you.","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":29,"context_line":"        super(OptionalTask, self).__init__(*args, **kwargs)"},{"line_number":30,"context_line":"        self.execute \u003d self.__catch_all(self.execute)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def __catch_all(self, func):"},{"line_number":33,"context_line":"        # NOTE(flaper87): Read this comment before calling the MI6"},{"line_number":34,"context_line":"        # Here\u0027s the thing, there\u0027s no nice way to define \"optional\""},{"line_number":35,"context_line":"        # tasks. That is, tasks whose failure shouldn\u0027t affect the execution"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_cf1971d2","line":32,"updated":"2015-03-03 23:34:28.000000000","message":"Some people realize this, some don\u0027t. Using double underscore will cause name-mangling by Python on new-style classes. Is there a reason you chose the double-underscore here?","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":29,"context_line":"        super(OptionalTask, self).__init__(*args, **kwargs)"},{"line_number":30,"context_line":"        self.execute \u003d self.__catch_all(self.execute)"},{"line_number":31,"context_line":""},{"line_number":32,"context_line":"    def __catch_all(self, func):"},{"line_number":33,"context_line":"        # NOTE(flaper87): Read this comment before calling the MI6"},{"line_number":34,"context_line":"        # Here\u0027s the thing, there\u0027s no nice way to define \"optional\""},{"line_number":35,"context_line":"        # tasks. That is, tasks whose failure shouldn\u0027t affect the execution"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_19770904","line":32,"in_reply_to":"ba7be1f8_cf1971d2","updated":"2015-03-04 13:52:36.000000000","message":"As I mentioned in a previous review, It\u0027s just to make extra emphasis on the fact that this should not be overwritten (or used) by anyone.\n\nChanged to a single `_` since you\u0027re like the  4th one asking why I used 2. :P","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":41,"context_line":"        # - Task\u0027s states will shamelessly lie to us saying the task succeeded."},{"line_number":42,"context_line":"        #"},{"line_number":43,"context_line":"        # - No revert procedure will be triggered, which means optional tasks,"},{"line_number":44,"context_line":"        # for now, mustn\u0027t cause any side-effects becayse they won\u0027t be able to"},{"line_number":45,"context_line":"        # clean them up. If these tasks depend on other task that do cause side"},{"line_number":46,"context_line":"        # effects, a task that cleans those side effects most be registered as"},{"line_number":47,"context_line":"        # well. For example, _ImportToFS, _MyDumbTask, _DeleteFromFS."}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_8f3cd930","line":44,"updated":"2015-03-03 23:34:28.000000000","message":"because","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"}],"glance/common/config.py":[{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":75,"context_line":"               help\u003d_(\u0027Work dir for asynchronous task operations. \u0027"},{"line_number":76,"context_line":"                      \u0027The directory set here will be used to operate over \u0027"},{"line_number":77,"context_line":"                      \u0027images - normally before they are imported in the \u0027"},{"line_number":78,"context_line":"                      \u0027destination store. When providing a work dir, make \u0027"},{"line_number":79,"context_line":"                      \u0027enough space is provided for concurrent tasks to run \u0027"},{"line_number":80,"context_line":"                      \u0027efficiently without running out of space. A rough \u0027"},{"line_number":81,"context_line":"                      \u0027estimation can be done by multiplying the number of \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_2f592d8e","line":78,"updated":"2015-03-03 23:34:28.000000000","message":"make sure enough space?","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":75,"context_line":"               help\u003d_(\u0027Work dir for asynchronous task operations. \u0027"},{"line_number":76,"context_line":"                      \u0027The directory set here will be used to operate over \u0027"},{"line_number":77,"context_line":"                      \u0027images - normally before they are imported in the \u0027"},{"line_number":78,"context_line":"                      \u0027destination store. When providing a work dir, make \u0027"},{"line_number":79,"context_line":"                      \u0027enough space is provided for concurrent tasks to run \u0027"},{"line_number":80,"context_line":"                      \u0027efficiently without running out of space. A rough \u0027"},{"line_number":81,"context_line":"                      \u0027estimation can be done by multiplying the number of \u0027"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_394bedb1","line":78,"in_reply_to":"ba7be1f8_2f592d8e","updated":"2015-03-04 13:52:36.000000000","message":"Done","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"0265456de849f8b9a7719a78e5f23413f58d4d64","unresolved":false,"context_lines":[{"line_number":70,"context_line":"               default\u003d\u0027taskflow\u0027,"},{"line_number":71,"context_line":"               help\u003d_(\"Specifies which task executor to be used to run the \""},{"line_number":72,"context_line":"                      \"task scripts.\")),"},{"line_number":73,"context_line":"    cfg.StrOpt(\u0027work_dir\u0027,"},{"line_number":74,"context_line":"               default\u003dNone,"},{"line_number":75,"context_line":"               help\u003d_(\u0027Work dir for asynchronous task operations. \u0027"},{"line_number":76,"context_line":"                      \u0027The directory set here will be used to operate over \u0027"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a80dd14_aaa5d4a2","line":73,"updated":"2015-03-10 13:09:29.000000000","message":"Could you update the example config file with this new option?","commit_id":"0d7c1a4722ab87528855c9a9ca74d2dbb04887e1"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"0265456de849f8b9a7719a78e5f23413f58d4d64","unresolved":false,"context_lines":[{"line_number":83,"context_line":"                      \u0027average image size (e.g 500MB). The image size \u0027"},{"line_number":84,"context_line":"                      \u0027estimation should be done based on the average size in \u0027"},{"line_number":85,"context_line":"                      \u0027your deployment. Note that depending on the tasks \u0027"},{"line_number":86,"context_line":"                      \u0027running you may need to multipy this number by some \u0027"},{"line_number":87,"context_line":"                      \u0027factor depending on what the task does. For example, \u0027"},{"line_number":88,"context_line":"                      \u0027you may want to double the available size if image \u0027"},{"line_number":89,"context_line":"                      \u0027conversion is enabled. All this being said, remember \u0027"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a80dd14_4abc58bb","line":86,"updated":"2015-03-10 13:09:29.000000000","message":"Typo: multiply","commit_id":"0d7c1a4722ab87528855c9a9ca74d2dbb04887e1"}],"glance/common/scripts/utils.py":[{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"baa70e92f7b808f93a610bcd06b4d0506cc871da","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    # verified before the task is created."},{"line_number":120,"context_line":"    if uri.startswith(\"file://\"):"},{"line_number":121,"context_line":"        uri \u003d uri.split(\"file://\")[-1]"},{"line_number":122,"context_line":"        return open(uri, \"r\").read()"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    return urllib2.urlopen(uri)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_03ae0fe6","line":122,"updated":"2015-02-25 13:42:45.000000000","message":"what happens to the file descriptor as we never close it?","commit_id":"e4e0d47f523663f2501879f5367c3fdaf8bd418b"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"f1c525127d5c29ce363c8b70a8ce31edb8b0ff72","unresolved":false,"context_lines":[{"line_number":119,"context_line":"    # verified before the task is created."},{"line_number":120,"context_line":"    if uri.startswith(\"file://\"):"},{"line_number":121,"context_line":"        uri \u003d uri.split(\"file://\")[-1]"},{"line_number":122,"context_line":"        return open(uri, \"r\").read()"},{"line_number":123,"context_line":""},{"line_number":124,"context_line":"    return urllib2.urlopen(uri)"}],"source_content_type":"text/x-python","patch_set":10,"id":"ba7be1f8_c60fa586","line":122,"in_reply_to":"ba7be1f8_03ae0fe6","updated":"2015-02-25 14:07:23.000000000","message":"the FD will be closed by python once the function\u0027s scope is cleaned up. The `read` call returns a reference to the content of the file, therefore the refcount will be dropped to 0 once this function returns. This will end up in the FD being closed.","commit_id":"e4e0d47f523663f2501879f5367c3fdaf8bd418b"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"d8cc4cd9942273da4c5a95062e9f7331f347b2b4","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    data. Note the absence of file:// for security reasons, see LP bug #942118."},{"line_number":116,"context_line":"    If the above constraint is violated, task should fail."},{"line_number":117,"context_line":"    \"\"\""},{"line_number":118,"context_line":"    # NOTE(flaper87): This is safe because the input uri is already"},{"line_number":119,"context_line":"    # verified before the task is created."},{"line_number":120,"context_line":"    if uri.startswith(\"file://\"):"},{"line_number":121,"context_line":"        uri \u003d uri.split(\"file://\")[-1]"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_badab914","line":118,"updated":"2015-02-25 17:09:18.000000000","message":"Why don\u0027t we use urllib2.urlopen directly?\nIt is a valid URI and urllib2 will process it correctly.\nIs there such a heavy penalty for going through urllib2 instead of open method?","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"2ee5f0dae652609e970d9ddc294ca0d9d5a2452d","unresolved":false,"context_lines":[{"line_number":115,"context_line":"    data. Note the absence of file:// for security reasons, see LP bug #942118."},{"line_number":116,"context_line":"    If the above constraint is violated, task should fail."},{"line_number":117,"context_line":"    \"\"\""},{"line_number":118,"context_line":"    # NOTE(flaper87): This is safe because the input uri is already"},{"line_number":119,"context_line":"    # verified before the task is created."},{"line_number":120,"context_line":"    if uri.startswith(\"file://\"):"},{"line_number":121,"context_line":"        uri \u003d uri.split(\"file://\")[-1]"}],"source_content_type":"text/x-python","patch_set":13,"id":"ba7be1f8_ca75dc32","line":118,"in_reply_to":"ba7be1f8_badab914","updated":"2015-02-25 18:17:32.000000000","message":"I literally had no idea that was possible..... Thanks for your input, I\u0027ll switch to just urllib2.\n\nThe penalty I mentioned below was related to loading things in memory, I believe urllib2 doesn\u0027t do that and it\u0027ll load things as you read the,","commit_id":"b1e56b3d642f1c2fa3a408e3f469ec6bde9ebba7"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":118,"context_line":"    # NOTE(flaper87): This is safe because the input uri is already"},{"line_number":119,"context_line":"    # verified before the task is created."},{"line_number":120,"context_line":"    if uri.startswith(\"file://\"):"},{"line_number":121,"context_line":"        uri \u003d uri.split(\"file://\")[-1]"},{"line_number":122,"context_line":"        # NOTE(flaper87): The caller of this function expects to have"},{"line_number":123,"context_line":"        # an iterable object. FileObjects in python are iterable, therefore"},{"line_number":124,"context_line":"        # we are returning it as is."}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_4feae174","line":121,"updated":"2015-03-03 23:34:28.000000000","message":"So can uri be a anything other than \n\n    file:///path/to/data\n\nIf not, why not just use:\n\n    uri \u003d uri.lstrip(\u0027file://\u0027) \n\nEspecially since we know it already starts with \u0027file://\u0027","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":118,"context_line":"    # NOTE(flaper87): This is safe because the input uri is already"},{"line_number":119,"context_line":"    # verified before the task is created."},{"line_number":120,"context_line":"    if uri.startswith(\"file://\"):"},{"line_number":121,"context_line":"        uri \u003d uri.split(\"file://\")[-1]"},{"line_number":122,"context_line":"        # NOTE(flaper87): The caller of this function expects to have"},{"line_number":123,"context_line":"        # an iterable object. FileObjects in python are iterable, therefore"},{"line_number":124,"context_line":"        # we are returning it as is."}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_d9af312a","line":121,"in_reply_to":"ba7be1f8_4feae174","updated":"2015-03-04 13:52:36.000000000","message":"Unfortunately, lstrip does not work as it should when given a URI scheme:\n\n    \u003e\u003e\u003e \"file:///missing/slash/\".lstrip(\u0027file://\u0027)\n    \u0027missing/slash/\u0027\n    \u003e\u003e\u003e\n\nThe above returned a string without the first slash, which breaks this. If there\u0027s something I don\u0027t know, lemme know. Otherwise, I think split is more than fine here. Both generate a copy of the original string, although I agree `lstrip` is more readable.","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        # an iterable object. FileObjects in python are iterable, therefore"},{"line_number":124,"context_line":"        # we are returning it as is."},{"line_number":125,"context_line":"        # The file descriptor will be eventually cleaned up by the garbage"},{"line_number":126,"context_line":"        # collector once its ref-count is dropped to 0. That is, when there"},{"line_number":127,"context_line":"        # wont be any references pointing to this file."},{"line_number":128,"context_line":"        #"},{"line_number":129,"context_line":"        # We\u0027re not using StringIO or other tools to avoid reading everything"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_ef9715ea","line":126,"updated":"2015-03-03 23:34:28.000000000","message":"Does glance support pypy? If so, this will cause problems for those who deploy on pypy. I\u0027ve seen file-descriptors grow out of control on pypy because of different gc configuration.","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"14238e18f972959080615d3e550e2a581bae55a0","unresolved":false,"context_lines":[{"line_number":123,"context_line":"        # an iterable object. FileObjects in python are iterable, therefore"},{"line_number":124,"context_line":"        # we are returning it as is."},{"line_number":125,"context_line":"        # The file descriptor will be eventually cleaned up by the garbage"},{"line_number":126,"context_line":"        # collector once its ref-count is dropped to 0. That is, when there"},{"line_number":127,"context_line":"        # wont be any references pointing to this file."},{"line_number":128,"context_line":"        #"},{"line_number":129,"context_line":"        # We\u0027re not using StringIO or other tools to avoid reading everything"}],"source_content_type":"text/x-python","patch_set":17,"id":"9a80dd14_39a76d51","line":126,"in_reply_to":"ba7be1f8_ef9715ea","updated":"2015-03-04 13:52:36.000000000","message":"no, it doesn\u0027t","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1769846edfeda83a569c6fc0747a5eb52f42e8af","unresolved":false,"context_lines":[{"line_number":117,"context_line":"    \"\"\""},{"line_number":118,"context_line":"    # NOTE(flaper87): This is safe because the input uri is already"},{"line_number":119,"context_line":"    # verified before the task is created."},{"line_number":120,"context_line":"    if uri.startswith(\"file://\"):"},{"line_number":121,"context_line":"        uri \u003d uri.split(\"file://\")[-1]"},{"line_number":122,"context_line":"        # NOTE(flaper87): The caller of this function expects to have"},{"line_number":123,"context_line":"        # an iterable object. FileObjects in python are iterable, therefore"}],"source_content_type":"text/x-python","patch_set":19,"id":"9a80dd14_36204ff7","line":120,"updated":"2015-03-04 17:30:27.000000000","message":"I saw Ian\u0027s comment, wouldn\u0027t it be beter doing it like:\n```\nprefix \u003d \u0027file://\u0027\nif uri.startswith(prefix):\n    uti \u003d uri[len(prefix):]\n```\nI don\u0027t really mind the way you do it though.","commit_id":"fddf099c75749489756b0bb10e54d65e033460b8"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"074b2edc547b054b747ccc8d4bb85d0cf7ad8c37","unresolved":false,"context_lines":[{"line_number":121,"context_line":"    # NOTE(flaper87): This is safe because the input uri is already"},{"line_number":122,"context_line":"    # verified before the task is created."},{"line_number":123,"context_line":"    if uri.startswith(\"file://\"):"},{"line_number":124,"context_line":"        uri \u003d uri.split(\"file://\")[-1]"},{"line_number":125,"context_line":"        # NOTE(flaper87): The caller of this function expects to have"},{"line_number":126,"context_line":"        # an iterable object. FileObjects in python are iterable, therefore"},{"line_number":127,"context_line":"        # we are returning it as is."}],"source_content_type":"text/x-python","patch_set":24,"id":"9a80dd14_7b5c87ee","line":124,"updated":"2015-03-10 15:10:32.000000000","message":"I think we can add a note here saying that this involved diskIOs and not memory. Traditional glance deployments involve larger memory flavors and may not have big disks. This needs to be pointed out if the number parallel tasks can be large enough to disrupt the behavior.","commit_id":"0503fdc2d81097d9453ce6ad844785fbb7905b79"},{"author":{"_account_id":5202,"name":"Erno Kuvaja","email":"jokke@usr.fi","username":"jokke"},"change_message_id":"743bab9cc2633023379379e8a4daecd85d33edce","unresolved":false,"context_lines":[{"line_number":121,"context_line":"    # NOTE(flaper87): This is safe because the input uri is already"},{"line_number":122,"context_line":"    # verified before the task is created."},{"line_number":123,"context_line":"    if uri.startswith(\"file://\"):"},{"line_number":124,"context_line":"        uri \u003d uri.split(\"file://\")[-1]"},{"line_number":125,"context_line":"        # NOTE(flaper87): The caller of this function expects to have"},{"line_number":126,"context_line":"        # an iterable object. FileObjects in python are iterable, therefore"},{"line_number":127,"context_line":"        # we are returning it as is."}],"source_content_type":"text/x-python","patch_set":24,"id":"9a80dd14_fc186d77","line":124,"in_reply_to":"9a80dd14_7b3f27d7","updated":"2015-03-10 15:54:09.000000000","message":"I do not agree that allowing file:// here does change the resource footprint. The impact of the tasks using filestore has nothing to do with glance.common.scripts.utils.get_image_data_iter() the change here just enables the use of this function within the tasks rather than reinventing the wheel.\n\nAs Flavio mentioned the impact is explained in the config comments, which is IMO the correct place to do so. Comment here would not gain basic deployers attention anyways.","commit_id":"0503fdc2d81097d9453ce6ad844785fbb7905b79"},{"author":{"_account_id":2537,"name":"Nikhil Komawar","email":"nik.komawar@gmail.com","username":"nikhil-komawar"},"change_message_id":"5f7ff5a2387e0ddacb4ef380e6b06ab7724ed0ee","unresolved":false,"context_lines":[{"line_number":121,"context_line":"    # NOTE(flaper87): This is safe because the input uri is already"},{"line_number":122,"context_line":"    # verified before the task is created."},{"line_number":123,"context_line":"    if uri.startswith(\"file://\"):"},{"line_number":124,"context_line":"        uri \u003d uri.split(\"file://\")[-1]"},{"line_number":125,"context_line":"        # NOTE(flaper87): The caller of this function expects to have"},{"line_number":126,"context_line":"        # an iterable object. FileObjects in python are iterable, therefore"},{"line_number":127,"context_line":"        # we are returning it as is."}],"source_content_type":"text/x-python","patch_set":24,"id":"9a80dd14_c159065d","line":124,"in_reply_to":"9a80dd14_7b3f27d7","updated":"2015-03-10 15:57:08.000000000","message":"Thanks, I was looking for doc/.. file however, the etc/.. changes work too.","commit_id":"0503fdc2d81097d9453ce6ad844785fbb7905b79"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"7a74ce3c87b7eff32eed12bf5583eb8120cfaf9b","unresolved":false,"context_lines":[{"line_number":121,"context_line":"    # NOTE(flaper87): This is safe because the input uri is already"},{"line_number":122,"context_line":"    # verified before the task is created."},{"line_number":123,"context_line":"    if uri.startswith(\"file://\"):"},{"line_number":124,"context_line":"        uri \u003d uri.split(\"file://\")[-1]"},{"line_number":125,"context_line":"        # NOTE(flaper87): The caller of this function expects to have"},{"line_number":126,"context_line":"        # an iterable object. FileObjects in python are iterable, therefore"},{"line_number":127,"context_line":"        # we are returning it as is."}],"source_content_type":"text/x-python","patch_set":24,"id":"9a80dd14_7b3f27d7","line":124,"in_reply_to":"9a80dd14_7b5c87ee","updated":"2015-03-10 15:28:03.000000000","message":"The usage of disks for the task workflow is explained in the `work_dir` config option. I believe that\u0027s a better place for it to be explained.","commit_id":"0503fdc2d81097d9453ce6ad844785fbb7905b79"}],"glance/domain/__init__.py":[{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"c57a73a2a4e570be4ea98b4a817fd85675149f9b","unresolved":false,"context_lines":[{"line_number":467,"context_line":"            # executor."},{"line_number":468,"context_line":"            task_executor \u003d CONF.task.task_executor"},{"line_number":469,"context_line":"            if task_executor \u003d\u003d \u0027eventlet\u0027:"},{"line_number":470,"context_line":"                task_executor \u003d \u0027taskflow\u0027"},{"line_number":471,"context_line":""},{"line_number":472,"context_line":"            executor_cls \u003d (\u0027glance.async.%s_executor.\u0027"},{"line_number":473,"context_line":"                            \u0027TaskExecutor\u0027 % task_executor)"}],"source_content_type":"text/x-python","patch_set":14,"id":"ba7be1f8_2ebd7ad8","line":470,"updated":"2015-02-26 17:21:33.000000000","message":"No deprecation warning?","commit_id":"c9d1d0edb53eeec52f9e87b76c059c775dcd9e2f"},{"author":{"_account_id":9535,"name":"Gorka Eguileor","email":"geguileo@redhat.com","username":"Gorka"},"change_message_id":"1769846edfeda83a569c6fc0747a5eb52f42e8af","unresolved":false,"context_lines":[{"line_number":469,"context_line":"            # executor."},{"line_number":470,"context_line":"            task_executor \u003d CONF.task.task_executor"},{"line_number":471,"context_line":"            if task_executor \u003d\u003d \u0027eventlet\u0027:"},{"line_number":472,"context_line":"                # NOTE(jokke): Making sure we do not log the deprecation"},{"line_number":473,"context_line":"                # warning 1000 times or anything crazy like that."},{"line_number":474,"context_line":"                if not TaskExecutorFactory.eventlet_deprecation_warned:"},{"line_number":475,"context_line":"                    msg \u003d _LW(\"The `eventlet` executor has been deprecated. \""}],"source_content_type":"text/x-python","patch_set":19,"id":"9a80dd14_561dc3bc","line":472,"updated":"2015-03-04 17:30:27.000000000","message":"Nice catch!! I didn\u0027t think about that  :)","commit_id":"fddf099c75749489756b0bb10e54d65e033460b8"}],"glance/tests/unit/async/flows/test_import.py":[{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"60676e5901940d0182fbd6bc3633fa8214d78d64","unresolved":false,"context_lines":[{"line_number":66,"context_line":"                             task_repo\u003dself.task_repo,"},{"line_number":67,"context_line":"                             image_repo\u003dself.img_repo,"},{"line_number":68,"context_line":"                             image_factory\u003dself.img_factory,"},{"line_number":69,"context_line":"                             uri\u003dmock.ANY)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def test_create_image(self):"},{"line_number":72,"context_line":"        image_create \u003d import_flow._CreateImage(self.task.task_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da86d52c_c819d924","line":69,"updated":"2015-02-16 17:10:58.000000000","message":"Is there an assertion that can be made here?","commit_id":"15bf927650fdf77d68d82aea0bee7ed363c2fd9a"},{"author":{"_account_id":6159,"name":"Flavio Percoco Premoli","display_name":"flaper87","email":"flavio.percoco@flyrlabs.com","username":"flaper87"},"change_message_id":"46b3151117c73c895f02f4c45aef328530ea4449","unresolved":false,"context_lines":[{"line_number":66,"context_line":"                             task_repo\u003dself.task_repo,"},{"line_number":67,"context_line":"                             image_repo\u003dself.img_repo,"},{"line_number":68,"context_line":"                             image_factory\u003dself.img_factory,"},{"line_number":69,"context_line":"                             uri\u003dmock.ANY)"},{"line_number":70,"context_line":""},{"line_number":71,"context_line":"    def test_create_image(self):"},{"line_number":72,"context_line":"        image_create \u003d import_flow._CreateImage(self.task.task_id,"}],"source_content_type":"text/x-python","patch_set":1,"id":"da86d52c_5b9d9956","line":69,"in_reply_to":"da86d52c_c819d924","updated":"2015-02-17 12:59:12.000000000","message":"ooopsss","commit_id":"15bf927650fdf77d68d82aea0bee7ed363c2fd9a"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"565df039556f28ec7a2cf9945d29523c62000d45","unresolved":false,"context_lines":[{"line_number":18,"context_line":"import StringIO"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":"import glance_store"},{"line_number":21,"context_line":"from oslo.config import cfg"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"import glance.async.flows.base_import as import_flow"},{"line_number":24,"context_line":"from glance.async import taskflow_executor"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_96695bfa","line":21,"updated":"2015-02-18 23:09:42.000000000","message":"Again oslo_config, no?","commit_id":"e54df7cf694b53938a6b124d58d6dd117e0e7629"},{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"43d3841221907d11379299f9160476abadc174c4","unresolved":false,"context_lines":[{"line_number":15,"context_line":""},{"line_number":16,"context_line":"import mock"},{"line_number":17,"context_line":"import os"},{"line_number":18,"context_line":"import StringIO"},{"line_number":19,"context_line":"import urllib2"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"import glance_store"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba7be1f8_efbe3564","line":18,"updated":"2015-03-03 23:34:28.000000000","message":"We can future-proof this by using the io module.","commit_id":"52eaaefb48c2f8c4bf213a0f2d88feb2325277da"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"0265456de849f8b9a7719a78e5f23413f58d4d64","unresolved":false,"context_lines":[{"line_number":222,"context_line":"                umock.assert_called_once()"},{"line_number":223,"context_line":""},{"line_number":224,"context_line":"                with open(image_path) as ifile:"},{"line_number":225,"context_line":"                    self.assertEqual(ifile.read(), content)"},{"line_number":226,"context_line":""},{"line_number":227,"context_line":"    def test_create_image(self):"},{"line_number":228,"context_line":"        image_create \u003d import_flow._CreateImage(self.task.task_id,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a80dd14_4a7ff8df","line":225,"updated":"2015-03-10 13:09:29.000000000","message":"We\u0027ve standardized the rest of assertEqual calls to have arguments in the order (expected, actual)","commit_id":"0d7c1a4722ab87528855c9a9ca74d2dbb04887e1"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"0265456de849f8b9a7719a78e5f23413f58d4d64","unresolved":false,"context_lines":[{"line_number":256,"context_line":"                save_image.execute(image.image_id)"},{"line_number":257,"context_line":"                get_mock.assert_called_once_with(image_id)"},{"line_number":258,"context_line":"                save_mock.assert_called_once_with(image)"},{"line_number":259,"context_line":"                self.assertEqual(\u0027active\u0027, image.status)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def test_import_to_fs(self):"},{"line_number":262,"context_line":"        import_fs \u003d import_flow._ImportToFS(self.task.task_id,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a80dd14_aa6f9429","line":259,"updated":"2015-03-10 13:09:29.000000000","message":"Same here","commit_id":"0d7c1a4722ab87528855c9a9ca74d2dbb04887e1"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"47339121e9b8b03d1afd9e3365632346552a7f9a","unresolved":false,"context_lines":[{"line_number":256,"context_line":"                save_image.execute(image.image_id)"},{"line_number":257,"context_line":"                get_mock.assert_called_once_with(image_id)"},{"line_number":258,"context_line":"                save_mock.assert_called_once_with(image)"},{"line_number":259,"context_line":"                self.assertEqual(\u0027active\u0027, image.status)"},{"line_number":260,"context_line":""},{"line_number":261,"context_line":"    def test_import_to_fs(self):"},{"line_number":262,"context_line":"        import_fs \u003d import_flow._ImportToFS(self.task.task_id,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a80dd14_457febdf","line":259,"in_reply_to":"9a80dd14_aa6f9429","updated":"2015-03-10 13:21:30.000000000","message":"Ignore this one.","commit_id":"0d7c1a4722ab87528855c9a9ca74d2dbb04887e1"}],"glance/tests/unit/async/flows/test_introspect.py":[{"author":{"_account_id":12000,"name":"Ian Cordasco","email":"sigmavirus24@gmail.com","username":"sigmavirus24"},"change_message_id":"565df039556f28ec7a2cf9945d29523c62000d45","unresolved":false,"context_lines":[{"line_number":17,"context_line":"import mock"},{"line_number":18,"context_line":""},{"line_number":19,"context_line":"import glance_store"},{"line_number":20,"context_line":"from oslo.config import cfg"},{"line_number":21,"context_line":"from oslo_concurrency import processutils"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"from glance.async.flows import introspect"}],"source_content_type":"text/x-python","patch_set":3,"id":"da86d52c_d625930a","line":20,"updated":"2015-02-18 23:09:42.000000000","message":"Shouldn\u0027t this be oslo_config?","commit_id":"e54df7cf694b53938a6b124d58d6dd117e0e7629"},{"author":{"_account_id":11356,"name":"Louis Taylor","email":"louis@kragniz.eu","username":"kragniz"},"change_message_id":"0265456de849f8b9a7719a78e5f23413f58d4d64","unresolved":false,"context_lines":[{"line_number":102,"context_line":"        # NOTE(flaper87): Don\u0027t mock, test the error."},{"line_number":103,"context_line":"        with mock.patch.object(processutils, \u0027execute\u0027) as exc_mock:"},{"line_number":104,"context_line":"            exc_mock.return_value \u003d (None, \"some error\")"},{"line_number":105,"context_line":"            # NOTE(flaper87): Pls, read the `OptionalTask.__catch_all`"},{"line_number":106,"context_line":"            # docs to know why this is commented."},{"line_number":107,"context_line":"            # self.assertRaises(RuntimeError,"},{"line_number":108,"context_line":"            #                  image_create.execute,"}],"source_content_type":"text/x-python","patch_set":23,"id":"9a80dd14_aa56b459","line":105,"updated":"2015-03-10 13:09:29.000000000","message":"You renamed __catch_all to only have one _ in a previous PS","commit_id":"0d7c1a4722ab87528855c9a9ca74d2dbb04887e1"}]}