)]}'
{"/COMMIT_MSG":[{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"4960404b155c1ea022e3e1c74a1d5aef84529263","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This new query parameter will allow to send a query sending the"},{"line_number":10,"context_line":"\"fields\" parameter. This \"fields\" parameter contains the needed"},{"line_number":11,"context_line":"OVO fields that require to be retrieved from the DB."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"As commented in the related bug, the OS client \"list\" command only"},{"line_number":14,"context_line":"prints five parameters, none of them the security group rules. In"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1fa4df85_90bd67b8","line":11,"range":{"start_line":11,"start_character":0,"end_line":11,"end_character":11},"updated":"2020-03-10 21:44:36.000000000","message":"nit: \"OVO fields\" is not visible to API users. Perhaps you would like to mean \"API fields\" converted into OVO fields inside neutron.","commit_id":"96500a04569422dd68a54d22998e79f438d2ef04"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1a6261089c6f47eceabb2779c5ef64d02c6b2a31","unresolved":false,"context_lines":[{"line_number":8,"context_line":""},{"line_number":9,"context_line":"This new query parameter will allow to send a query sending the"},{"line_number":10,"context_line":"\"fields\" parameter. This \"fields\" parameter contains the needed"},{"line_number":11,"context_line":"OVO fields that require to be retrieved from the DB."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"As commented in the related bug, the OS client \"list\" command only"},{"line_number":14,"context_line":"prints five parameters, none of them the security group rules. In"}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1fa4df85_74b43ff3","line":11,"range":{"start_line":11,"start_character":0,"end_line":11,"end_character":11},"in_reply_to":"1fa4df85_90bd67b8","updated":"2020-03-12 11:44:02.000000000","message":"You are right, we don\u0027t expose the OVO fields but what is defined in the API. I\u0027ll change it.","commit_id":"96500a04569422dd68a54d22998e79f438d2ef04"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"4960404b155c1ea022e3e1c74a1d5aef84529263","unresolved":false,"context_lines":[{"line_number":10,"context_line":"\"fields\" parameter. This \"fields\" parameter contains the needed"},{"line_number":11,"context_line":"OVO fields that require to be retrieved from the DB."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"As commented in the related bug, the OS client \"list\" command only"},{"line_number":14,"context_line":"prints five parameters, none of them the security group rules. In"},{"line_number":15,"context_line":"systems with a reasonable amount of security groups, skipping the"},{"line_number":16,"context_line":"unnecessary rule load can save a lot of time."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1fa4df85_30b033dd","line":13,"range":{"start_line":13,"start_character":37,"end_line":13,"end_character":46},"updated":"2020-03-10 21:44:36.000000000","message":"nit: we usually use OSC rather than OS client.","commit_id":"96500a04569422dd68a54d22998e79f438d2ef04"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1a6261089c6f47eceabb2779c5ef64d02c6b2a31","unresolved":false,"context_lines":[{"line_number":10,"context_line":"\"fields\" parameter. This \"fields\" parameter contains the needed"},{"line_number":11,"context_line":"OVO fields that require to be retrieved from the DB."},{"line_number":12,"context_line":""},{"line_number":13,"context_line":"As commented in the related bug, the OS client \"list\" command only"},{"line_number":14,"context_line":"prints five parameters, none of them the security group rules. In"},{"line_number":15,"context_line":"systems with a reasonable amount of security groups, skipping the"},{"line_number":16,"context_line":"unnecessary rule load can save a lot of time."}],"source_content_type":"text/x-gerrit-commit-message","patch_set":3,"id":"1fa4df85_b4bab7de","line":13,"range":{"start_line":13,"start_character":37,"end_line":13,"end_character":46},"in_reply_to":"1fa4df85_30b033dd","updated":"2020-03-12 11:44:02.000000000","message":"Right!","commit_id":"96500a04569422dd68a54d22998e79f438d2ef04"}],"openstackclient/network/v2/security_group.py":[{"author":{"_account_id":15554,"name":"Bence Romsics","email":"bence.romsics@gmail.com","username":"ebenrom","status":"working for Ericsson, UTC+1 (+DST)"},"change_message_id":"29e8bfd3083437bbbe09315740c7369cf381e244","unresolved":false,"context_lines":[{"line_number":202,"context_line":"# the OSC minimum requirements include SDK 1.0."},{"line_number":203,"context_line":"class ListSecurityGroup(common.NetworkAndComputeLister):"},{"line_number":204,"context_line":"    _description \u003d _(\"List security groups\")"},{"line_number":205,"context_line":"    LIST_FIELDS \u003d {\u0027id\u0027, \u0027name\u0027, \u0027description\u0027, \u0027project_id\u0027, \u0027tags\u0027}"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def update_parser_network(self, parser):"},{"line_number":208,"context_line":"        if not self.is_docs_build:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1fa4df85_10a325b4","line":205,"updated":"2020-03-03 16:12:52.000000000","message":"Why all uppercase?\n\nThe use of a set here works, but it trips up openstackclient\u0027s debug printing (openstack --debug). See the url in this example:\n\nREQ: curl -g -i -X GET \"http://192.168.122.169:9696/v2.0/security-groups?fields\u003d%7B%27id%27%2C+%27name%27%2C+%27description%27%2C+%27tags%27%2C+%27project_id%27%7D\" -H \"Accept: application/json\" -H \"User-Agent: openstacksdk/0.40.1 keystoneauth1/3.18.0 python-requests/2.22.0 CPython/3.6.9\" -H \"X-Auth-Token: {SHA256}f020d0396d8918b0b8a084902b4182daa82492d4dda1bc9a0dec5506e7e64100\"\n\nWhile the actual URL on the wire is good:\n\nGET /v2.0/security-groups?fields\u003did\u0026fields\u003dname\u0026fields\u003ddescription\u0026fields\u003dtags\u0026fields\u003dproject_id HTTP/1.1.","commit_id":"6940191fdee726cc188dfbdd9a05ad5a1dc8e638"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ed1060a962335ca3423282383e40fe324e0a3034","unresolved":false,"context_lines":[{"line_number":202,"context_line":"# the OSC minimum requirements include SDK 1.0."},{"line_number":203,"context_line":"class ListSecurityGroup(common.NetworkAndComputeLister):"},{"line_number":204,"context_line":"    _description \u003d _(\"List security groups\")"},{"line_number":205,"context_line":"    LIST_FIELDS \u003d {\u0027id\u0027, \u0027name\u0027, \u0027description\u0027, \u0027project_id\u0027, \u0027tags\u0027}"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def update_parser_network(self, parser):"},{"line_number":208,"context_line":"        if not self.is_docs_build:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1fa4df85_70293999","line":205,"in_reply_to":"1fa4df85_10a325b4","updated":"2020-03-03 16:40:10.000000000","message":"I defined this in uppercase because this is a class constant.\n\nMaybe I should use a list, but I think both work. Other reviewers for sure will help us on this.","commit_id":"6940191fdee726cc188dfbdd9a05ad5a1dc8e638"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"6b232b002d6730c29db3f603e741e2c489c99b23","unresolved":false,"context_lines":[{"line_number":202,"context_line":"# the OSC minimum requirements include SDK 1.0."},{"line_number":203,"context_line":"class ListSecurityGroup(common.NetworkAndComputeLister):"},{"line_number":204,"context_line":"    _description \u003d _(\"List security groups\")"},{"line_number":205,"context_line":"    LIST_FIELDS \u003d {\u0027id\u0027, \u0027name\u0027, \u0027description\u0027, \u0027project_id\u0027, \u0027tags\u0027}"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def update_parser_network(self, parser):"},{"line_number":208,"context_line":"        if not self.is_docs_build:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1fa4df85_62c91c57","line":205,"in_reply_to":"1fa4df85_33bad9c0","updated":"2020-03-09 15:29:32.000000000","message":"Done","commit_id":"6940191fdee726cc188dfbdd9a05ad5a1dc8e638"},{"author":{"_account_id":2,"name":"Monty Taylor","email":"mordred@inaugust.com","username":"mordred"},"change_message_id":"bf1d3bb99d65a7da6259fe1a7412a65bbe0bb901","unresolved":false,"context_lines":[{"line_number":202,"context_line":"# the OSC minimum requirements include SDK 1.0."},{"line_number":203,"context_line":"class ListSecurityGroup(common.NetworkAndComputeLister):"},{"line_number":204,"context_line":"    _description \u003d _(\"List security groups\")"},{"line_number":205,"context_line":"    LIST_FIELDS \u003d {\u0027id\u0027, \u0027name\u0027, \u0027description\u0027, \u0027project_id\u0027, \u0027tags\u0027}"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def update_parser_network(self, parser):"},{"line_number":208,"context_line":"        if not self.is_docs_build:"}],"source_content_type":"text/x-python","patch_set":2,"id":"1fa4df85_33bad9c0","line":205,"in_reply_to":"1fa4df85_70293999","updated":"2020-03-04 14:11:32.000000000","message":"Yeah - let\u0027s use a list for the debug printing (we could also fix the debug printing because I think a set is nice here - but that\u0027s handled in keystoneauth and that seems like maybe a lot of work for this case.","commit_id":"6940191fdee726cc188dfbdd9a05ad5a1dc8e638"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"ed1060a962335ca3423282383e40fe324e0a3034","unresolved":false,"context_lines":[{"line_number":240,"context_line":"        return parser"},{"line_number":241,"context_line":""},{"line_number":242,"context_line":"    def take_action_network(self, client, parsed_args):"},{"line_number":243,"context_line":"        fields \u003d self.LIST_FIELDS"},{"line_number":244,"context_line":"        filters \u003d {}"},{"line_number":245,"context_line":"        if parsed_args.project:"},{"line_number":246,"context_line":"            identity_client \u003d self.app.client_manager.identity"}],"source_content_type":"text/x-python","patch_set":2,"id":"1fa4df85_f0168955","line":243,"updated":"2020-03-03 16:40:10.000000000","message":"Uhmmm, in PS2 now this is unnecessary.","commit_id":"6940191fdee726cc188dfbdd9a05ad5a1dc8e638"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"4960404b155c1ea022e3e1c74a1d5aef84529263","unresolved":false,"context_lines":[{"line_number":202,"context_line":"# the OSC minimum requirements include SDK 1.0."},{"line_number":203,"context_line":"class ListSecurityGroup(common.NetworkAndComputeLister):"},{"line_number":204,"context_line":"    _description \u003d _(\"List security groups\")"},{"line_number":205,"context_line":"    LIST_FIELDS \u003d [\u0027id\u0027, \u0027name\u0027, \u0027description\u0027, \u0027project_id\u0027, \u0027tags\u0027]"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def update_parser_network(self, parser):"},{"line_number":208,"context_line":"        if not self.is_docs_build:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1fa4df85_d0143feb","line":205,"updated":"2020-03-10 21:44:36.000000000","message":"This is specific to take_action_network. It looks better to use a name which clarifies it is used in case of \"network\" (i.e. \"neutron\") service. How about is \"FIELDS_NETWORK\"?\n(It is inside ListSecurityGroup class, so \"LIST_\" looks unnecessary.)\n\nnote: I first thought it is better to move it to take_action_network, but it is used in the unit tests, so it looks better to explore a better name.","commit_id":"96500a04569422dd68a54d22998e79f438d2ef04"},{"author":{"_account_id":841,"name":"Akihiro Motoki","email":"amotoki@gmail.com","username":"amotoki"},"change_message_id":"3a05ed547ea24403152da5c87cea91477eb8c29e","unresolved":false,"context_lines":[{"line_number":202,"context_line":"# the OSC minimum requirements include SDK 1.0."},{"line_number":203,"context_line":"class ListSecurityGroup(common.NetworkAndComputeLister):"},{"line_number":204,"context_line":"    _description \u003d _(\"List security groups\")"},{"line_number":205,"context_line":"    LIST_FIELDS \u003d [\u0027id\u0027, \u0027name\u0027, \u0027description\u0027, \u0027project_id\u0027, \u0027tags\u0027]"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def update_parser_network(self, parser):"},{"line_number":208,"context_line":"        if not self.is_docs_build:"}],"source_content_type":"text/x-python","patch_set":3,"id":"df33271e_fc132465","line":205,"in_reply_to":"1fa4df85_34024740","updated":"2020-03-24 12:37:46.000000000","message":"(just a comment) OSC still covers both nova-network and neutron. Thus, this class is not necessarily under network.v2 unfortunately. That\u0027s the detail reason I suggested including \"NETWORK\".\n\n(I think patch set 4 is clear enough, so I am fine with patch set 4. This is just a comment to clarify the situation of OSC.)","commit_id":"96500a04569422dd68a54d22998e79f438d2ef04"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"1a6261089c6f47eceabb2779c5ef64d02c6b2a31","unresolved":false,"context_lines":[{"line_number":202,"context_line":"# the OSC minimum requirements include SDK 1.0."},{"line_number":203,"context_line":"class ListSecurityGroup(common.NetworkAndComputeLister):"},{"line_number":204,"context_line":"    _description \u003d _(\"List security groups\")"},{"line_number":205,"context_line":"    LIST_FIELDS \u003d [\u0027id\u0027, \u0027name\u0027, \u0027description\u0027, \u0027project_id\u0027, \u0027tags\u0027]"},{"line_number":206,"context_line":""},{"line_number":207,"context_line":"    def update_parser_network(self, parser):"},{"line_number":208,"context_line":"        if not self.is_docs_build:"}],"source_content_type":"text/x-python","patch_set":3,"id":"1fa4df85_34024740","line":205,"in_reply_to":"1fa4df85_d0143feb","updated":"2020-03-12 11:44:02.000000000","message":"I agree that \"LIST_\" is unnecessary being a class variable inside \"ListSecurityGroup\"\n\nBut in \"FIELDS_NETWORK\", \"_NETWORK\" looks unnecessary too. This is a network command and it\u0027s inside a class under network.v2.\n\nIMO, a good descriptive name could be \"FIELDS_TO_RETRIEVE\". This is accurate because we are going to retrieve only those API field parameters (the API call will return only those fields).","commit_id":"96500a04569422dd68a54d22998e79f438d2ef04"}]}
