)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":1994,"name":"Zhongyue Luo","email":"zhongyue.luo@gmail.com","username":"zyluo"},"unresolved":false,"context_lines":[{"line_number":4,"context_line":"Commit:     Juergen Brendel \u003cjbrendel@cisco.com\u003e"},{"line_number":5,"context_line":"CommitDate: 2012-11-12 14:35:03 +1300"},{"line_number":6,"context_line":""},{"line_number":7,"context_line":"Use DB count to get resource counts. Fixes bug 1075369."},{"line_number":8,"context_line":""},{"line_number":9,"context_line":"During quota check we used to simply retrieve the entire collection"},{"line_number":10,"context_line":"of resources from the database, then counting them in Python. This"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"AAAAMH%2F%2F%2F70%3D","line":7,"updated":"2012-11-12 01:53:56.000000000","message":"Actually \"Fixes bug 1075369\" should be placed above \"Change-Id\" according to http://wiki.openstack.org/GitCommitMessages","commit_id":"89730e777a428d2ead2f73cb96be170097692719"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":10,"context_line":""},{"line_number":11,"context_line":"During quota check we used to simply retrieve the entire collection"},{"line_number":12,"context_line":"of resources from the database, then counting them in Python. This"},{"line_number":13,"context_line":"patch introduces an additional flag to the _get_collection-count()"},{"line_number":14,"context_line":"method, which instead take advantage of the DB\u0027s built-in count"},{"line_number":15,"context_line":"capabilities."},{"line_number":16,"context_line":""}],"source_content_type":"text/x-gerrit-commit-message","patch_set":5,"id":"AAAAMH%2F%2F%2BoI%3D","line":13,"updated":"2012-11-13 08:32:47.000000000","message":"_get_collection-count -\u003e _get_collection_count (s/-/_/)","commit_id":"fb0f1cb7bc3784a5fa6965749a2f2859933c3e25"}],"quantum/db/db_base_plugin_v2.py":[{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"change_message_id":"676e9963f49ff1260f87acbdcbf39bc9a9f870c3","unresolved":false,"context_lines":[{"line_number":213,"context_line":"        if count_only:"},{"line_number":214,"context_line":"            return collection.count()"},{"line_number":215,"context_line":"        else:"},{"line_number":216,"context_line":"            return [dict_func(c, fields) for c in collection.all()]"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    @staticmethod"},{"line_number":219,"context_line":"    def _generate_mac(context, network_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2FusQ%3D","line":216,"updated":"2012-11-09 17:48:02.000000000","message":"why overload _get_collection() rather than have a separate _get_collection_count()?  It does save you anything in terms of simplifying the callers of this function, and I think its bad style to have a function that has two different return types (int vs. list) based on a parameter.","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"unresolved":false,"context_lines":[{"line_number":213,"context_line":"        if count_only:"},{"line_number":214,"context_line":"            return collection.count()"},{"line_number":215,"context_line":"        else:"},{"line_number":216,"context_line":"            return [dict_func(c, fields) for c in collection.all()]"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    @staticmethod"},{"line_number":219,"context_line":"    def _generate_mac(context, network_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2Ft%2Bs%3D","line":216,"in_reply_to":"AAAALn%2F%2FupY%3D","updated":"2012-11-10 07:27:18.000000000","message":"I prefer to Dan\u0027s suggestion. It not only two different types and params \u0027fields\u0027 is useful when count_only\u003dFalse, unuseful when count_only\u003dTrue.\n\nHow about:\ndef _get_collection_query(...)\n     collection \u003d self._model_query(context, model)\n     collection \u003d self._apply_filters_to_query(collection, model, filters)\n\ndef _get_collection(...)\n     query \u003d _get_collection_query()\n     return query.all()\n\ndef _get_collection_count(...)\n     query \u003d _get_collection_query()\n     return query.count()","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":6139,"name":"Juergen Brendel","email":"jbrendel@cisco.com","username":"jbrendel"},"change_message_id":"e21d8703608ecc11acbd7d1e047e6669818d4ca3","unresolved":false,"context_lines":[{"line_number":213,"context_line":"        if count_only:"},{"line_number":214,"context_line":"            return collection.count()"},{"line_number":215,"context_line":"        else:"},{"line_number":216,"context_line":"            return [dict_func(c, fields) for c in collection.all()]"},{"line_number":217,"context_line":""},{"line_number":218,"context_line":"    @staticmethod"},{"line_number":219,"context_line":"    def _generate_mac(context, network_id):"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2FupY%3D","line":216,"in_reply_to":"AAAALn%2F%2FusQ%3D","updated":"2012-11-09 18:45:09.000000000","message":"This one already had the proper way to apply filters and I wanted to just be able to reuse it. This is the ONE function that knows how to do this, so why not? Otherwise, I\u0027d either have to factor out the \u0027collection-getting\u0027 into yet another function, or I would have to replicate that code in the get_collection_count() function.\n\nIt\u0027s also an internal, semi-private function, so I have less concern about the different return types. Only we use it here so it stays well contained.","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"change_message_id":"676e9963f49ff1260f87acbdcbf39bc9a9f870c3","unresolved":false,"context_lines":[{"line_number":937,"context_line":"        return self._get_networks(context, filters, fields, count_only\u003dFalse)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":"    def get_networks_count(self, context, filters\u003dNone, fields\u003dNone):"},{"line_number":940,"context_line":"        return self._get_networks(context, filters, fields, count_only\u003dTrue)"},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"    def create_subnet_bulk(self, context, subnets):"},{"line_number":943,"context_line":"        return self._create_bulk(\u0027subnet\u0027, context, subnets)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2FusA%3D","line":940,"updated":"2012-11-09 17:48:02.000000000","message":"I don\u0027 think you need to introduce _get_networks() if you just use a separate _count_collection() method as suggested above.","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":6139,"name":"Juergen Brendel","email":"jbrendel@cisco.com","username":"jbrendel"},"change_message_id":"e21d8703608ecc11acbd7d1e047e6669818d4ca3","unresolved":false,"context_lines":[{"line_number":937,"context_line":"        return self._get_networks(context, filters, fields, count_only\u003dFalse)"},{"line_number":938,"context_line":""},{"line_number":939,"context_line":"    def get_networks_count(self, context, filters\u003dNone, fields\u003dNone):"},{"line_number":940,"context_line":"        return self._get_networks(context, filters, fields, count_only\u003dTrue)"},{"line_number":941,"context_line":""},{"line_number":942,"context_line":"    def create_subnet_bulk(self, context, subnets):"},{"line_number":943,"context_line":"        return self._create_bulk(\u0027subnet\u0027, context, subnets)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2Fuo8%3D","line":940,"in_reply_to":"AAAALn%2F%2FusA%3D","updated":"2012-11-09 18:45:09.000000000","message":"Separate _count_collection() function or not, I could have called the function directly. My thinking was if we need to prepare something ahead of getting the network collection (maybe fiddling with the filters some more or some such thing), then we can do this in the _get_networks() function, before going off to accessing the collection. It\u0027s not needed here right now, but I felt it would be nice to have a single jump-off point that knows to do the right things to access a network collection.\n\nIf it bothers you, I can change it back and make calls to _get_collection() directly in the get_networks() and get_networks_count() functions.","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":1994,"name":"Zhongyue Luo","email":"zhongyue.luo@gmail.com","username":"zyluo"},"unresolved":false,"context_lines":[{"line_number":212,"context_line":"        return collection"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def _get_collection(self, context, model, dict_func, filters\u003dNone,"},{"line_number":215,"context_line":"                        fields\u003dNone, count_only\u003dFalse):"},{"line_number":216,"context_line":"        query \u003d self._get_collection_query(context, model, dict_func, filters)"},{"line_number":217,"context_line":"        return [dict_func(c, fields) for c in query.all()]"},{"line_number":218,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAMH%2F%2F%2F9M%3D","line":215,"updated":"2012-11-12 01:18:14.000000000","message":"Where is count_only used?","commit_id":"37ea149d0428847cbb41720ed1adc46a5999cea5"},{"author":{"_account_id":6139,"name":"Juergen Brendel","email":"jbrendel@cisco.com","username":"jbrendel"},"change_message_id":"e21d8703608ecc11acbd7d1e047e6669818d4ca3","unresolved":false,"context_lines":[{"line_number":212,"context_line":"        return collection"},{"line_number":213,"context_line":""},{"line_number":214,"context_line":"    def _get_collection(self, context, model, dict_func, filters\u003dNone,"},{"line_number":215,"context_line":"                        fields\u003dNone, count_only\u003dFalse):"},{"line_number":216,"context_line":"        query \u003d self._get_collection_query(context, model, dict_func, filters)"},{"line_number":217,"context_line":"        return [dict_func(c, fields) for c in query.all()]"},{"line_number":218,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"AAAAMH%2F%2F%2F80%3D","line":215,"in_reply_to":"AAAAMH%2F%2F%2F9M%3D","updated":"2012-11-12 01:33:10.000000000","message":"Oops, I forgot to remove that one. Will do and upload patch.","commit_id":"37ea149d0428847cbb41720ed1adc46a5999cea5"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":216,"context_line":"        query \u003d self._get_collection_query(context, model, dict_func, filters)"},{"line_number":217,"context_line":"        return [dict_func(c, fields) for c in query.all()]"},{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def _get_collection_count(self, context, model, dict_func, filters\u003dNone):"},{"line_number":220,"context_line":"        query \u003d self._get_collection_query(context, model, dict_func, filters)"},{"line_number":221,"context_line":"        return query.count()"},{"line_number":222,"context_line":""}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAMH%2F%2F%2FI8%3D","line":219,"updated":"2012-11-12 20:25:32.000000000","message":"dict_func doesn\u0027t make sense as an input here, as we aren\u0027t actually getting results.","commit_id":"89730e777a428d2ead2f73cb96be170097692719"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":938,"context_line":""},{"line_number":939,"context_line":"    def get_networks_count(self, context, filters\u003dNone):"},{"line_number":940,"context_line":"        return self._get_collection_count(context, models_v2.Network,"},{"line_number":941,"context_line":"                                          self._make_network_dict,"},{"line_number":942,"context_line":"                                          filters\u003dfilters)"},{"line_number":943,"context_line":""},{"line_number":944,"context_line":"    def create_subnet_bulk(self, context, subnets):"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAMH%2F%2F%2FIM%3D","line":941,"updated":"2012-11-12 20:25:32.000000000","message":"ditto about dict_func here and below.","commit_id":"89730e777a428d2ead2f73cb96be170097692719"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":218,"context_line":""},{"line_number":219,"context_line":"    def _get_collection_count(self, context, model, filters\u003dNone):"},{"line_number":220,"context_line":"        query \u003d self._get_collection_query(context, model, filters)"},{"line_number":221,"context_line":"        return query.count()"},{"line_number":222,"context_line":""},{"line_number":223,"context_line":"    @staticmethod"},{"line_number":224,"context_line":"    def _generate_mac(context, network_id):"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAMH%2F%2F%2Bog%3D","line":221,"updated":"2012-11-13 08:32:47.000000000","message":"return self._get_collection_query(context, model, filters).count()\n\nJust same as marc\u0027s comment for a port.","commit_id":"fb0f1cb7bc3784a5fa6965749a2f2859933c3e25"},{"author":{"_account_id":2592,"name":"Mark McClain","email":"mark@mcclain.xyz","username":"markmcclain"},"unresolved":false,"context_lines":[{"line_number":1289,"context_line":"        return [self._make_port_dict(c, fields) for c in query.all()]"},{"line_number":1290,"context_line":""},{"line_number":1291,"context_line":"    def get_ports_count(self, context, filters\u003dNone):"},{"line_number":1292,"context_line":"        query \u003d self._get_ports_query(context, filters)"},{"line_number":1293,"context_line":"        return query.count()"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAMH%2F%2F%2BvE%3D","line":1292,"updated":"2012-11-13 05:39:54.000000000","message":"return self._get_ports_query(context, filters).count()","commit_id":"fb0f1cb7bc3784a5fa6965749a2f2859933c3e25"}],"quantum/db/l3_db.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"unresolved":false,"context_lines":[{"line_number":252,"context_line":"                                    self._make_router_dict,"},{"line_number":253,"context_line":"                                    filters\u003dfilters, fields\u003dfields)"},{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    def get_routers_count(self, context, filters\u003dNone, fields\u003dNone):"},{"line_number":256,"context_line":"        return self._get_collection(context, Router,"},{"line_number":257,"context_line":"                                    self._make_router_dict,"},{"line_number":258,"context_line":"                                    filters\u003dfilters, fields\u003dfields,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2Ft%2Bk%3D","line":255,"updated":"2012-11-10 07:27:18.000000000","message":"params \u0027fields\u0027 is unuseful for count","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"unresolved":false,"context_lines":[{"line_number":622,"context_line":"                                    self._make_floatingip_dict,"},{"line_number":623,"context_line":"                                    filters\u003dfilters, fields\u003dfields)"},{"line_number":624,"context_line":""},{"line_number":625,"context_line":"    def get_floatingips_count(self, context, filters\u003dNone, fields\u003dNone):"},{"line_number":626,"context_line":"        return self._get_collection(context, FloatingIP,"},{"line_number":627,"context_line":"                                    self._make_floatingip_dict,"},{"line_number":628,"context_line":"                                    filters\u003dfilters, fields\u003dfields,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2Ft%2Bg%3D","line":625,"updated":"2012-11-10 07:27:18.000000000","message":"same with above","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":254,"context_line":""},{"line_number":255,"context_line":"    def get_routers_count(self, context, filters\u003dNone):"},{"line_number":256,"context_line":"        return self._get_collection_count(context, Router,"},{"line_number":257,"context_line":"                                          self._make_router_dict,"},{"line_number":258,"context_line":"                                          filters\u003dfilters)"},{"line_number":259,"context_line":""},{"line_number":260,"context_line":"    def _check_for_dup_router_subnet(self, context, router_id,"}],"source_content_type":"text/x-python","patch_set":3,"id":"AAAAMH%2F%2F%2FIc%3D","line":257,"updated":"2012-11-12 20:25:32.000000000","message":"same here","commit_id":"89730e777a428d2ead2f73cb96be170097692719"}],"quantum/db/securitygroups_db.py":[{"author":{"_account_id":5754,"name":"Alex Xu","email":"hejie.xu@intel.com","username":"xuhj"},"unresolved":false,"context_lines":[{"line_number":150,"context_line":"                                    self._make_security_group_dict,"},{"line_number":151,"context_line":"                                    filters\u003dfilters, fields\u003dfields)"},{"line_number":152,"context_line":""},{"line_number":153,"context_line":"    def get_security_groups_count(self, context, filters\u003dNone, fields\u003dNone):"},{"line_number":154,"context_line":"        return self._get_collection(context, SecurityGroup,"},{"line_number":155,"context_line":"                                    self._make_security_group_dict,"},{"line_number":156,"context_line":"                                    filters\u003dfilters, fields\u003dfields,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2Ft%2Bc%3D","line":153,"updated":"2012-11-10 07:27:18.000000000","message":"fields is unuseful","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":386,"context_line":"    def get_security_group_rules(self, context, filters\u003dNone, fields\u003dNone):"},{"line_number":387,"context_line":"        return self._get_collection(context, SecurityGroupRule,"},{"line_number":388,"context_line":"                                    self._make_security_group_rule_dict,"},{"line_number":389,"context_line":"                                    filters\u003dfilters, fields\u003dfields)"},{"line_number":390,"context_line":""},{"line_number":391,"context_line":"    def get_security_group_rule(self, context, id, fields\u003dNone):"},{"line_number":392,"context_line":"        security_group_rule \u003d self._get_security_group_rule(context, id)"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAMH%2F%2F%2Bok%3D","line":389,"updated":"2012-11-13 08:32:47.000000000","message":"security group extension also defines a quota for securitry_group_rules. We need to add get_security_group_rules_count().","commit_id":"fb0f1cb7bc3784a5fa6965749a2f2859933c3e25"}],"quantum/plugins/linuxbridge/lb_quantum_plugin.py":[{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"change_message_id":"676e9963f49ff1260f87acbdcbf39bc9a9f870c3","unresolved":false,"context_lines":[{"line_number":403,"context_line":"        if count_only:"},{"line_number":404,"context_line":"            # If the count_only flag is set then the get_networks() function"},{"line_number":405,"context_line":"            # of the superclass will just have returned the count. Nothing"},{"line_number":406,"context_line":"            # further to do, time to return."},{"line_number":407,"context_line":"            return nets"},{"line_number":408,"context_line":"        for net in nets:"},{"line_number":409,"context_line":"            self._extend_network_dict_provider(context, net)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2Fur0%3D","line":406,"updated":"2012-11-09 17:48:02.000000000","message":"I believe this issue goes away if you make the changes I suggest in the base db class, as you don\u0027t modify the get_networks() path at all.","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":6139,"name":"Juergen Brendel","email":"jbrendel@cisco.com","username":"jbrendel"},"change_message_id":"e21d8703608ecc11acbd7d1e047e6669818d4ca3","unresolved":false,"context_lines":[{"line_number":403,"context_line":"        if count_only:"},{"line_number":404,"context_line":"            # If the count_only flag is set then the get_networks() function"},{"line_number":405,"context_line":"            # of the superclass will just have returned the count. Nothing"},{"line_number":406,"context_line":"            # further to do, time to return."},{"line_number":407,"context_line":"            return nets"},{"line_number":408,"context_line":"        for net in nets:"},{"line_number":409,"context_line":"            self._extend_network_dict_provider(context, net)"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2FuqE%3D","line":406,"in_reply_to":"AAAALn%2F%2Fur0%3D","updated":"2012-11-09 18:45:09.000000000","message":"If I change get_networks() to always return a count then sure, I don\u0027t need to change anything here, but I also removed a means to just get the objects.\n\nBut get_networks() may be used by someone who really want to just get the entire collection. That\u0027s why I want to use a specialized count method if I know I want the count, only falling back to retrieving the full collection and doing a Python count only if this method is no available.\n\nI contemplated adding a \u0027count_only\u0027 parameter to get_networks(), but then I get a public method that has two different return types. As you pointed out in another comment, that\u0027s not always nice. I don\u0027t normally mind for private functions, though.","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"}],"quantum/quota.py":[{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"change_message_id":"676e9963f49ff1260f87acbdcbf39bc9a9f870c3","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        # An optimized function to get the count of a resource is not"},{"line_number":284,"context_line":"        # available. Need to do it manually: Get all the objects and"},{"line_number":285,"context_line":"        # then count them. Not good for performance, but helps to"},{"line_number":286,"context_line":"        # support un-optimized plugins and resources."},{"line_number":287,"context_line":"        obj_getter \u003d getattr(plugin, \"get_%s\" % resources)"},{"line_number":288,"context_line":"        obj_list \u003d obj_getter(context, filters\u003d{\u0027tenant_id\u0027: [tenant_id]})"},{"line_number":289,"context_line":"        return len(obj_list) if obj_list else 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2Furc%3D","line":286,"updated":"2012-11-09 17:48:02.000000000","message":"these comments seem more verbose than necessary.  A simple comment like \"use optimized resource count methods if supported by plugin, otherwise fall back to fetching all items and counting (less efficient)\" would probably suffice.","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":6139,"name":"Juergen Brendel","email":"jbrendel@cisco.com","username":"jbrendel"},"change_message_id":"e21d8703608ecc11acbd7d1e047e6669818d4ca3","unresolved":false,"context_lines":[{"line_number":283,"context_line":"        # An optimized function to get the count of a resource is not"},{"line_number":284,"context_line":"        # available. Need to do it manually: Get all the objects and"},{"line_number":285,"context_line":"        # then count them. Not good for performance, but helps to"},{"line_number":286,"context_line":"        # support un-optimized plugins and resources."},{"line_number":287,"context_line":"        obj_getter \u003d getattr(plugin, \"get_%s\" % resources)"},{"line_number":288,"context_line":"        obj_list \u003d obj_getter(context, filters\u003d{\u0027tenant_id\u0027: [tenant_id]})"},{"line_number":289,"context_line":"        return len(obj_list) if obj_list else 0"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2FuqY%3D","line":286,"in_reply_to":"AAAALn%2F%2Furc%3D","updated":"2012-11-09 18:45:09.000000000","message":"Sure, maybe I\u0027m just trying to compensate for the general lack of comments that I see. I tend to be verbose in my comments. If that is not the style here then I will try to shorten it.","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"}],"quantum/tests/unit/test_api_v2.py":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":756,"context_line":""},{"line_number":757,"context_line":"        return_value \u003d {\u0027id\u0027: net_id, \u0027status\u0027: \"ACTIVE\"}"},{"line_number":758,"context_line":"        return_value.update(full_input[\u0027network\u0027])"},{"line_number":759,"context_line":"        return_networks \u003d {\u0027networks\u0027: [return_value]}"},{"line_number":760,"context_line":"        instance \u003d self.plugin.return_value"},{"line_number":761,"context_line":"        instance.get_networks.return_value \u003d return_networks"},{"line_number":762,"context_line":"        instance.get_networks_count.return_value \u003d 1"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAMH%2F%2F5p4%3D","line":759,"updated":"2012-11-16 14:12:55.000000000","message":"Should the value of return_networks a list \"[return_value]\"\n(though it is not a part of this patch)?\n\nget_networks() of plugin interface is expected to return a list of networks.","commit_id":"2de33c49cb2a1daec3ecacf41cc2d60d74bef988"},{"author":{"_account_id":6139,"name":"Juergen Brendel","email":"jbrendel@cisco.com","username":"jbrendel"},"change_message_id":"e21d8703608ecc11acbd7d1e047e6669818d4ca3","unresolved":false,"context_lines":[{"line_number":756,"context_line":""},{"line_number":757,"context_line":"        return_value \u003d {\u0027id\u0027: net_id, \u0027status\u0027: \"ACTIVE\"}"},{"line_number":758,"context_line":"        return_value.update(full_input[\u0027network\u0027])"},{"line_number":759,"context_line":"        return_networks \u003d {\u0027networks\u0027: [return_value]}"},{"line_number":760,"context_line":"        instance \u003d self.plugin.return_value"},{"line_number":761,"context_line":"        instance.get_networks.return_value \u003d return_networks"},{"line_number":762,"context_line":"        instance.get_networks_count.return_value \u003d 1"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAMH%2F%2F5QY%3D","line":759,"in_reply_to":"AAAAMH%2F%2F5p4%3D","updated":"2012-11-16 18:29:52.000000000","message":"get_networks() is not called anymore for quota check when a new network is created. Therefore, the return value for get_networks() does not need to be specified anymore. In fact, both the return_value and return_networks variables are not needed anymore.","commit_id":"2de33c49cb2a1daec3ecacf41cc2d60d74bef988"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":779,"context_line":"            return_value.update(full_input[\u0027network\u0027])"},{"line_number":780,"context_line":"            return_networks.append(return_value)"},{"line_number":781,"context_line":"        self.assertEquals(3, len(return_networks))"},{"line_number":782,"context_line":"        instance \u003d self.plugin.return_value"},{"line_number":783,"context_line":"        instance.get_networks.return_value \u003d return_networks"},{"line_number":784,"context_line":"        res \u003d self.api.post_json("},{"line_number":785,"context_line":"            _get_path(\u0027networks\u0027), initial_input)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAMH%2F%2F5qA%3D","line":782,"updated":"2012-11-16 14:12:55.000000000","message":"Don\u0027t we need to add the following to pass unittests in SmokeStack?\n\ninstace.get_networks_count.return_value \u003d 3","commit_id":"2de33c49cb2a1daec3ecacf41cc2d60d74bef988"},{"author":{"_account_id":6139,"name":"Juergen Brendel","email":"jbrendel@cisco.com","username":"jbrendel"},"change_message_id":"e21d8703608ecc11acbd7d1e047e6669818d4ca3","unresolved":false,"context_lines":[{"line_number":779,"context_line":"            return_value.update(full_input[\u0027network\u0027])"},{"line_number":780,"context_line":"            return_networks.append(return_value)"},{"line_number":781,"context_line":"        self.assertEquals(3, len(return_networks))"},{"line_number":782,"context_line":"        instance \u003d self.plugin.return_value"},{"line_number":783,"context_line":"        instance.get_networks.return_value \u003d return_networks"},{"line_number":784,"context_line":"        res \u003d self.api.post_json("},{"line_number":785,"context_line":"            _get_path(\u0027networks\u0027), initial_input)"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAMH%2F%2F5QI%3D","line":782,"in_reply_to":"AAAAMH%2F%2F5qA%3D","updated":"2012-11-16 18:29:52.000000000","message":"This test does not actually create multiple networks. We just want to check that the single network create works, even though there are some networks already. Since we only use get_networks_count() for quota check, it is enough to mock the return value of get_networks_count() (as you had pointed out), but we can remove the entire construction of the mock return_networks list.","commit_id":"2de33c49cb2a1daec3ecacf41cc2d60d74bef988"}],"quantum/tests/unit/test_db_plugin.py":[{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"change_message_id":"676e9963f49ff1260f87acbdcbf39bc9a9f870c3","unresolved":false,"context_lines":[{"line_number":393,"context_line":"        yield network"},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"        if do_delete:"},{"line_number":396,"context_line":"            self._delete(\u0027networks\u0027, network[\u0027network\u0027][\u0027id\u0027])"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    @contextlib.contextmanager"},{"line_number":399,"context_line":"    def subnet(self, network\u003dNone,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2FurU%3D","line":396,"updated":"2012-11-09 17:48:02.000000000","message":"were you intending to use this change somewhere in the patchset?  I don\u0027t see it.","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":393,"context_line":"        yield network"},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"        if do_delete:"},{"line_number":396,"context_line":"            self._delete(\u0027networks\u0027, network[\u0027network\u0027][\u0027id\u0027])"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    @contextlib.contextmanager"},{"line_number":399,"context_line":"    def subnet(self, network\u003dNone,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAAMH%2F%2F%2FH0%3D","line":396,"in_reply_to":"AAAALn%2F%2Fup4%3D","updated":"2012-11-12 20:24:32.000000000","message":"Its fine, its just that from a review perspective it made it look like perhaps you had forgotten to commit some new test code that used this capability.","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":6139,"name":"Juergen Brendel","email":"jbrendel@cisco.com","username":"jbrendel"},"change_message_id":"e21d8703608ecc11acbd7d1e047e6669818d4ca3","unresolved":false,"context_lines":[{"line_number":393,"context_line":"        yield network"},{"line_number":394,"context_line":""},{"line_number":395,"context_line":"        if do_delete:"},{"line_number":396,"context_line":"            self._delete(\u0027networks\u0027, network[\u0027network\u0027][\u0027id\u0027])"},{"line_number":397,"context_line":""},{"line_number":398,"context_line":"    @contextlib.contextmanager"},{"line_number":399,"context_line":"    def subnet(self, network\u003dNone,"}],"source_content_type":"text/x-python","patch_set":1,"id":"AAAALn%2F%2Fup4%3D","line":396,"in_reply_to":"AAAALn%2F%2FurU%3D","updated":"2012-11-09 18:45:09.000000000","message":"Yes, I had written a unit test that checked for performance degradation when many networks were created. Eventually I took that unit test out, since it ran for a while. However, adding this parameter here might not be so bad, since it allows similar tests (or tests that need multiple networks) to be written using this function.\n\nI know, it\u0027s not directly related, but would it be bad if we leave it in?","commit_id":"1a14ced123bd3467e55cee8c014d42d482759944"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"unresolved":false,"context_lines":[{"line_number":392,"context_line":"        yield network"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        if do_delete:"},{"line_number":395,"context_line":"            self._delete(\u0027networks\u0027, network[\u0027network\u0027][\u0027id\u0027])"},{"line_number":396,"context_line":""},{"line_number":397,"context_line":"    @contextlib.contextmanager"},{"line_number":398,"context_line":"    def subnet(self, network\u003dNone,"}],"source_content_type":"text/x-python","patch_set":5,"id":"AAAAMH%2F%2F%2BoQ%3D","line":395,"updated":"2012-11-13 08:32:47.000000000","message":"I think it is worth adding why you added this as described in your response to Dan in patch set 1 review.","commit_id":"fb0f1cb7bc3784a5fa6965749a2f2859933c3e25"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"unresolved":false,"context_lines":[{"line_number":376,"context_line":"    def network(self, name\u003d\u0027net1\u0027,"},{"line_number":377,"context_line":"                admin_status_up\u003dTrue,"},{"line_number":378,"context_line":"                fmt\u003d\u0027json\u0027,"},{"line_number":379,"context_line":"                do_delete\u003dTrue,"},{"line_number":380,"context_line":"                **kwargs):"},{"line_number":381,"context_line":"        res \u003d self._create_network(fmt,"},{"line_number":382,"context_line":"                                   name,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAMH%2F%2F8Do%3D","line":379,"updated":"2012-11-15 01:03:04.000000000","message":"Since u are asserting all of current network() method calls should delete network, why d u have such parameter?\n\nI cannot see you are using do_delete \u003d False. correct me if I am missing something.","commit_id":"2de33c49cb2a1daec3ecacf41cc2d60d74bef988"},{"author":{"_account_id":447,"name":"dan wendlandt","email":"dan@nicira.com","username":"danwent"},"unresolved":false,"context_lines":[{"line_number":376,"context_line":"    def network(self, name\u003d\u0027net1\u0027,"},{"line_number":377,"context_line":"                admin_status_up\u003dTrue,"},{"line_number":378,"context_line":"                fmt\u003d\u0027json\u0027,"},{"line_number":379,"context_line":"                do_delete\u003dTrue,"},{"line_number":380,"context_line":"                **kwargs):"},{"line_number":381,"context_line":"        res \u003d self._create_network(fmt,"},{"line_number":382,"context_line":"                                   name,"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAMH%2F%2F7H0%3D","line":379,"in_reply_to":"AAAAMH%2F%2F8Do%3D","updated":"2012-11-15 17:49:57.000000000","message":"yeah, i had a similar comment early in the review.  He ended up writing test code that used this, but not submitting it, but thought it was a useful thing to keep in.  I could go either way on this.","commit_id":"2de33c49cb2a1daec3ecacf41cc2d60d74bef988"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"unresolved":false,"context_lines":[{"line_number":392,"context_line":"        yield network"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        if do_delete:"},{"line_number":395,"context_line":"            # The do_delete attribute allows you to control whether the"},{"line_number":396,"context_line":"            # created network is immediately deleted agaon. Therefore, this"},{"line_number":397,"context_line":"            # function is also usable in tests, which require the creation"},{"line_number":398,"context_line":"            # of many networks."}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAMH%2F%2F8DA%3D","line":395,"updated":"2012-11-15 01:03:04.000000000","message":"attribute -\u003e parameter ?","commit_id":"2de33c49cb2a1daec3ecacf41cc2d60d74bef988"},{"author":{"_account_id":6139,"name":"Juergen Brendel","email":"jbrendel@cisco.com","username":"jbrendel"},"change_message_id":"e21d8703608ecc11acbd7d1e047e6669818d4ca3","unresolved":false,"context_lines":[{"line_number":392,"context_line":"        yield network"},{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        if do_delete:"},{"line_number":395,"context_line":"            # The do_delete attribute allows you to control whether the"},{"line_number":396,"context_line":"            # created network is immediately deleted agaon. Therefore, this"},{"line_number":397,"context_line":"            # function is also usable in tests, which require the creation"},{"line_number":398,"context_line":"            # of many networks."}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAMH%2F%2F5QE%3D","line":395,"in_reply_to":"AAAAMH%2F%2F8DA%3D","updated":"2012-11-16 18:29:52.000000000","message":"Done","commit_id":"2de33c49cb2a1daec3ecacf41cc2d60d74bef988"},{"author":{"_account_id":2874,"name":"yong sheng gong","email":"gong.yongsheng@99cloud.net","username":"gongysh"},"unresolved":false,"context_lines":[{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        if do_delete:"},{"line_number":395,"context_line":"            # The do_delete attribute allows you to control whether the"},{"line_number":396,"context_line":"            # created network is immediately deleted agaon. Therefore, this"},{"line_number":397,"context_line":"            # function is also usable in tests, which require the creation"},{"line_number":398,"context_line":"            # of many networks."},{"line_number":399,"context_line":"            self._delete(\u0027networks\u0027, network[\u0027network\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAMH%2F%2F8C8%3D","line":396,"updated":"2012-11-15 01:03:04.000000000","message":"agaon?","commit_id":"2de33c49cb2a1daec3ecacf41cc2d60d74bef988"},{"author":{"_account_id":6139,"name":"Juergen Brendel","email":"jbrendel@cisco.com","username":"jbrendel"},"change_message_id":"e21d8703608ecc11acbd7d1e047e6669818d4ca3","unresolved":false,"context_lines":[{"line_number":393,"context_line":""},{"line_number":394,"context_line":"        if do_delete:"},{"line_number":395,"context_line":"            # The do_delete attribute allows you to control whether the"},{"line_number":396,"context_line":"            # created network is immediately deleted agaon. Therefore, this"},{"line_number":397,"context_line":"            # function is also usable in tests, which require the creation"},{"line_number":398,"context_line":"            # of many networks."},{"line_number":399,"context_line":"            self._delete(\u0027networks\u0027, network[\u0027network\u0027][\u0027id\u0027])"}],"source_content_type":"text/x-python","patch_set":6,"id":"AAAAMH%2F%2F5P8%3D","line":396,"in_reply_to":"AAAAMH%2F%2F8C8%3D","updated":"2012-11-16 18:29:52.000000000","message":"Done","commit_id":"2de33c49cb2a1daec3ecacf41cc2d60d74bef988"}]}