)]}'
{"id":"openstack%2Fpython-mistralclient~371234","triplet_id":"openstack%2Fpython-mistralclient~master~If6ff35e91c3d35c2741332c7e739bb92b1234c54","project":"openstack/python-mistralclient","branch":"master","topic":"bp/mistral-abstract-auth","hashtags":[],"change_id":"If6ff35e91c3d35c2741332c7e739bb92b1234c54","subject":"Abstract authentication function","status":"MERGED","created":"2016-09-16 01:48:19.000000000","updated":"2016-10-05 06:48:49.000000000","submitted":"2016-09-26 06:05:14.000000000","submitter":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"total_comment_count":2,"unresolved_comment_count":0,"has_review_started":true,"meta_rev_id":"9cd6900b46585143073acc611335223e1382bd79","_number":371234,"virtual_id_number":371234,"owner":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"actions":{},"labels":{"Verified":{"approved":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"all":[{"value":0,"date":"2016-09-29 09:55:36.000000000","post_submit":true,"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},{"value":0,"date":"2016-09-26 12:39:51.000000000","post_submit":true,"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"},{"value":2,"date":"2016-09-26 06:05:14.000000000","post_submit":true,"_account_id":3,"name":"Jenkins","username":"jenkins"},{"value":0,"_account_id":7700,"name":"Nikolay Mahotkin","email":"nikolay.makhotkin@gmail.com","username":"nmakhotkin"},{"value":0,"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},{"value":0,"date":"2016-09-26 19:11:59.000000000","post_submit":true,"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},{"value":0,"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},{"value":0,"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"}],"values":{"-2":"Fails","-1":"Doesn\u0027t seem to work"," 0":"No score","+1":"Works for me","+2":"Verified"},"description":"","default_value":0,"optional":true},"Code-Review":{"approved":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"all":[{"value":0,"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},{"value":0,"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"},{"value":0,"_account_id":3,"name":"Jenkins","username":"jenkins"},{"value":0,"_account_id":7700,"name":"Nikolay Mahotkin","email":"nikolay.makhotkin@gmail.com","username":"nmakhotkin"},{"value":0,"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},{"value":0,"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},{"value":2,"date":"2016-09-25 12:29:43.000000000","permitted_voting_range":{"min":2,"max":2},"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},{"value":2,"date":"2016-09-26 05:28:03.000000000","_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"}],"values":{"-2":"Do not merge","-1":"This patch needs further work before it can be merged"," 0":"No score","+1":"Looks good to me, but someone else must approve","+2":"Looks good to me (core reviewer)"},"description":"","default_value":0,"optional":true},"Workflow":{"approved":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"all":[{"value":0,"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},{"value":0,"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"},{"value":0,"_account_id":3,"name":"Jenkins","username":"jenkins"},{"value":0,"_account_id":7700,"name":"Nikolay Mahotkin","email":"nikolay.makhotkin@gmail.com","username":"nmakhotkin"},{"value":0,"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},{"value":0,"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},{"value":0,"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},{"value":1,"date":"2016-09-26 05:28:03.000000000","_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"}],"values":{"-1":"Work in progress"," 0":"Ready for reviews","+1":"Approved"},"description":"","default_value":0,"optional":true}},"removable_reviewers":[],"reviewers":{"REVIEWER":[{"_account_id":3,"name":"Jenkins","username":"jenkins"},{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},{"_account_id":7700,"name":"Nikolay Mahotkin","email":"nikolay.makhotkin@gmail.com","username":"nmakhotkin"},{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},{"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"}]},"pending_reviewers":{},"reviewer_updates":[{"updated":"2016-09-16 01:49:39.000000000","updated_by":{"_account_id":7700,"name":"Nikolay Mahotkin","email":"nikolay.makhotkin@gmail.com","username":"nmakhotkin"},"reviewer":{"_account_id":7700,"name":"Nikolay Mahotkin","email":"nikolay.makhotkin@gmail.com","username":"nmakhotkin"},"state":"REVIEWER"},{"updated":"2016-09-19 11:44:12.000000000","updated_by":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"reviewer":{"_account_id":19206,"name":"Dawid Deja","email":"dawdej@gmail.com","username":"ddeja"},"state":"REVIEWER"},{"updated":"2016-09-25 12:29:43.000000000","updated_by":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"reviewer":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"state":"REVIEWER"},{"updated":"2016-09-26 05:28:03.000000000","updated_by":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"reviewer":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"state":"REVIEWER"},{"updated":"2016-09-26 06:05:14.000000000","updated_by":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"reviewer":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"state":"REVIEWER"},{"updated":"2016-09-26 12:39:51.000000000","updated_by":{"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"},"reviewer":{"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"},"state":"REVIEWER"},{"updated":"2016-09-29 09:55:36.000000000","updated_by":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"reviewer":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"state":"REVIEWER"}],"messages":[{"id":"dc8afb05c483a5e5783868eaacb59c775a699ccd","author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"date":"2016-09-16 01:48:19.000000000","message":"Uploaded patch set 1.","accounts_in_message":[],"_revision_number":1},{"id":"935aa3fbb043de72b5e68e60f96b2c8df696a9da","author":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"date":"2016-09-16 02:15:36.000000000","message":"Patch Set 1: Verified+1\n\nBuild succeeded (check pipeline).\n\n- gate-python-mistralclient-docs-ubuntu-xenial http://docs-draft.openstack.org/34/371234/1/check/gate-python-mistralclient-docs-ubuntu-xenial/20257f2//doc/build/html/ : SUCCESS in 2m 13s\n- gate-python-mistralclient-pep8-ubuntu-xenial http://logs.openstack.org/34/371234/1/check/gate-python-mistralclient-pep8-ubuntu-xenial/1a21ce5/ : SUCCESS in 2m 17s\n- gate-python-mistralclient-python27-ubuntu-xenial http://logs.openstack.org/34/371234/1/check/gate-python-mistralclient-python27-ubuntu-xenial/0ba1006/ : SUCCESS in 2m 24s\n- gate-python-mistralclient-python34 http://logs.openstack.org/34/371234/1/check/gate-python-mistralclient-python34/a9b399d/ : SUCCESS in 2m 16s\n- gate-python-mistralclient-python35-nv http://logs.openstack.org/34/371234/1/check/gate-python-mistralclient-python35-nv/d8d4c56/ : SUCCESS in 2m 40s (non-voting)\n- gate-python-mistralclient-requirements http://logs.openstack.org/34/371234/1/check/gate-python-mistralclient-requirements/e2ba4f0/ : SUCCESS in 5m 51s\n- check-osc-plugins http://logs.openstack.org/34/371234/1/check/check-osc-plugins/3eb7fba/ : SUCCESS in 3m 26s (non-voting)\n- gate-python-mistralclient-devstack-dsvm http://logs.openstack.org/34/371234/1/check/gate-python-mistralclient-devstack-dsvm/e171ec3/ : SUCCESS in 24m 39s","accounts_in_message":[],"_revision_number":1},{"id":"118bf7b9556b795e3d9c2a39ba589fae11586057","author":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"date":"2016-09-19 07:16:36.000000000","message":"Patch Set 1: Code-Review+1\n\n(1 comment)\n\nSeems Good !!","accounts_in_message":[],"_revision_number":1},{"id":"1de64f8942a1a38ba05e73dd92f4759be3550284","author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"date":"2016-09-19 17:28:02.000000000","message":"Patch Set 1:\n\n(1 comment)","accounts_in_message":[],"_revision_number":1},{"id":"9eb6b55679dff9885db6316a9be83b00a42bc4ca","author":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"date":"2016-09-20 05:14:32.000000000","message":"Patch Set 1: Code-Review+2","accounts_in_message":[],"_revision_number":1},{"id":"bebc24bbdb98dba694c1a0e3e0aa98854654d6ae","author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"date":"2016-09-21 13:11:20.000000000","message":"Patch Set 1: Code-Review+1\n\nLooks very good to me, nice job. I just want to test this with a real Keyclock installation first to make sure that everything is ok. After that I\u0027ll give +2. The change is too important.\n\nThanks","accounts_in_message":[],"_revision_number":1},{"id":"db61ad010ba9423e3d480eedf734bda998a8c50a","author":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"date":"2016-09-21 13:17:37.000000000","message":"Patch Set 1: Verified-1\n\nMerge Failed.\n\nThis change or one of its cross-repo dependencies was unable to be automatically merged with the current state of its repository. Please rebase the change and upload a new patchset.","accounts_in_message":[],"_revision_number":1},{"id":"e569d43f60cf446b326a5f7a6d0eda7b1bda9334","author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"date":"2016-09-21 23:19:40.000000000","message":"Uploaded patch set 2.","accounts_in_message":[],"_revision_number":2},{"id":"87c0391faaf184e9a358f9c14999465ff83bfe7e","author":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"date":"2016-09-21 23:41:14.000000000","message":"Patch Set 2: Verified-1\n\nBuild failed (check pipeline).  For information on how to proceed, see http://docs.openstack.org/infra/manual/developers.html#automated-testing\n\n- gate-python-mistralclient-docs-ubuntu-xenial http://docs-draft.openstack.org/34/371234/2/check/gate-python-mistralclient-docs-ubuntu-xenial/92c2d26//doc/build/html/ : SUCCESS in 4m 34s\n- gate-python-mistralclient-pep8-ubuntu-xenial http://logs.openstack.org/34/371234/2/check/gate-python-mistralclient-pep8-ubuntu-xenial/79e0888/ : SUCCESS in 2m 38s\n- gate-python-mistralclient-python27-ubuntu-xenial http://logs.openstack.org/34/371234/2/check/gate-python-mistralclient-python27-ubuntu-xenial/487e4ea/ : SUCCESS in 2m 16s\n- gate-python-mistralclient-python34 http://logs.openstack.org/34/371234/2/check/gate-python-mistralclient-python34/7328856/ : SUCCESS in 2m 20s\n- gate-python-mistralclient-python35-nv http://logs.openstack.org/34/371234/2/check/gate-python-mistralclient-python35-nv/6c87aef/ : SUCCESS in 5m 31s (non-voting)\n- gate-python-mistralclient-requirements http://logs.openstack.org/34/371234/2/check/gate-python-mistralclient-requirements/53da2cb/ : SUCCESS in 2m 25s\n- check-osc-plugins http://logs.openstack.org/34/371234/2/check/check-osc-plugins/c9eeef6/ : SUCCESS in 4m 40s (non-voting)\n- gate-python-mistralclient-devstack-dsvm http://logs.openstack.org/34/371234/2/check/gate-python-mistralclient-devstack-dsvm/8a1ea46/ : FAILURE in 20m 24s","accounts_in_message":[],"_revision_number":2},{"id":"b2a19a23a947822f67754d655061da0e30089c5b","author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"date":"2016-09-22 01:01:24.000000000","message":"Uploaded patch set 3.","accounts_in_message":[],"_revision_number":3},{"id":"f3607a6baa4dfaf27d01dc13cd156741cc63bcb2","author":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"date":"2016-09-22 01:45:19.000000000","message":"Patch Set 3: Verified+1\n\nBuild succeeded (check pipeline).\n\n- gate-python-mistralclient-docs-ubuntu-xenial http://docs-draft.openstack.org/34/371234/3/check/gate-python-mistralclient-docs-ubuntu-xenial/06a475a//doc/build/html/ : SUCCESS in 3m 06s\n- gate-python-mistralclient-pep8-ubuntu-xenial http://logs.openstack.org/34/371234/3/check/gate-python-mistralclient-pep8-ubuntu-xenial/8f6ce9a/ : SUCCESS in 2m 59s\n- gate-python-mistralclient-python27-ubuntu-xenial http://logs.openstack.org/34/371234/3/check/gate-python-mistralclient-python27-ubuntu-xenial/976e35e/ : SUCCESS in 2m 16s\n- gate-python-mistralclient-python34 http://logs.openstack.org/34/371234/3/check/gate-python-mistralclient-python34/060b2e7/ : SUCCESS in 5m 08s\n- gate-python-mistralclient-python35-nv http://logs.openstack.org/34/371234/3/check/gate-python-mistralclient-python35-nv/7e14034/ : SUCCESS in 2m 52s (non-voting)\n- gate-python-mistralclient-requirements http://logs.openstack.org/34/371234/3/check/gate-python-mistralclient-requirements/8e79181/ : SUCCESS in 1m 55s\n- check-osc-plugins http://logs.openstack.org/34/371234/3/check/check-osc-plugins/8d10f42/ : SUCCESS in 4m 52s (non-voting)\n- gate-python-mistralclient-devstack-dsvm http://logs.openstack.org/34/371234/3/check/gate-python-mistralclient-devstack-dsvm/c66e258/ : SUCCESS in 42m 54s","accounts_in_message":[],"_revision_number":3},{"id":"8fa06d1951689f9258cb05e092146d1a8e8304f9","author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"date":"2016-09-25 12:29:43.000000000","message":"Patch Set 3: Code-Review+2","accounts_in_message":[],"_revision_number":3},{"id":"b02f7670f75e2aed37ca2e7f8479b398556e72a8","author":{"_account_id":18238,"name":"Hardik Parekh","email":"hardik.parekh047@gmail.com","username":"hparekh"},"date":"2016-09-26 05:28:03.000000000","message":"Patch Set 3: Code-Review+2 Workflow+1","accounts_in_message":[],"_revision_number":3},{"id":"b4b22515126bca002aa003d255e8d15f0c5a8c41","author":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"date":"2016-09-26 05:28:13.000000000","message":"Patch Set 3: -Verified\n\nStarting gate jobs.\nhttp://status.openstack.org/zuul/","accounts_in_message":[],"_revision_number":3},{"id":"9b18ee4d0f5ff3a0611a2adcf91fd83dce79b93b","date":"2016-09-26 06:05:14.000000000","message":"Change has been successfully merged into the git repository by Jenkins","accounts_in_message":[],"_revision_number":3},{"id":"fdaad813034b384d81d7f1957b65b4567cd4d928","author":{"_account_id":3,"name":"Jenkins","username":"jenkins"},"date":"2016-09-26 06:05:14.000000000","message":"Patch Set 3: Verified+2\n\nBuild succeeded (gate pipeline).\n\n- gate-python-mistralclient-docs-ubuntu-xenial http://docs-draft.openstack.org/34/371234/3/gate/gate-python-mistralclient-docs-ubuntu-xenial/b4b82ba//doc/build/html/ : SUCCESS in 2m 56s\n- gate-python-mistralclient-pep8-ubuntu-xenial http://logs.openstack.org/34/371234/3/gate/gate-python-mistralclient-pep8-ubuntu-xenial/38d2d9e/ : SUCCESS in 3m 55s\n- gate-python-mistralclient-python27-ubuntu-xenial http://logs.openstack.org/34/371234/3/gate/gate-python-mistralclient-python27-ubuntu-xenial/4893ff7/ : SUCCESS in 2m 10s\n- gate-python-mistralclient-python34 http://logs.openstack.org/34/371234/3/gate/gate-python-mistralclient-python34/a379727/ : SUCCESS in 3m 03s\n- gate-python-mistralclient-requirements http://logs.openstack.org/34/371234/3/gate/gate-python-mistralclient-requirements/bcbb6fd/ : SUCCESS in 2m 43s\n- gate-python-mistralclient-devstack-dsvm http://logs.openstack.org/34/371234/3/gate/gate-python-mistralclient-devstack-dsvm/ceb2418/ : SUCCESS in 36m 52s","accounts_in_message":[],"_revision_number":3},{"id":"5b509d606e8a01c86b6ec0b64658f31b098b36ee","author":{"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"},"date":"2016-09-26 12:39:51.000000000","message":"Patch Set 3:\n\nI\u0027m unfortunately, late to come here but I strongly disagree with the changes in this patch. It should not be general to use kw args everywhere. It disables code insight and analysis. Is it possible to revert this change?","accounts_in_message":[],"_revision_number":3},{"id":"9b3f74f63c09a2a61abfb6bf47729ef9089ceb28","author":{"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"},"date":"2016-09-26 12:55:04.000000000","message":"Patch Set 3: Reverted\n\nThis patchset was reverted in change: Iddb72df6eb63eeb70f9d031bf3d41f68606d9bb1","accounts_in_message":[],"_revision_number":3},{"id":"f52144ad985ca2eca14ad08450b60d60e4cba9c1","author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"date":"2016-09-26 19:11:59.000000000","message":"Patch Set 3:\n\nInstead of just reverting the change. Is there a recommendation on how to implement?","accounts_in_message":[],"_revision_number":3},{"id":"99619a989935924a737e7b126cd52876ec2d9e66","author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"date":"2016-09-27 05:29:58.000000000","message":"Patch Set 3:\n\nRegarding using kwargs, I would really prefer to avoid them. For HttpClient in this patch I would just use normal arguments. For cases when we have 10+ arguments it seems to be a usual Python practice to use kwargs and have a docstring that describes all parameters. This docstring also helps with code assist also. This approach seems to be reasonable because in our Client class we had a huge number of arguments and it was a mess, too. So I think Winson made a good change here.\n\nNo need to revert the whole change, let\u0027s improve only places that we still don\u0027t like.","accounts_in_message":[],"_revision_number":3},{"id":"c75e5bb53cbe7de1f76af94e5fa30d4e1d3a520b","author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"date":"2016-09-27 05:33:12.000000000","message":"Patch Set 3:\n\nI looked again and I suggest we add a docstring for client classes (except HttpClient where we can just use normal parameters). It should help.","accounts_in_message":[],"_revision_number":3},{"id":"2f350a15cebc2e1bd29a3e533e923328c1079ce4","author":{"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"},"date":"2016-09-27 07:11:43.000000000","message":"Patch Set 3:\n\n\u003e I looked again and I suggest we add a docstring for client classes\n \u003e (except HttpClient where we can just use normal parameters). It\n \u003e should help.","accounts_in_message":[],"_revision_number":3},{"id":"982df973842c7ec231c3a92da6733249ff31c459","author":{"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"},"date":"2016-09-27 07:34:05.000000000","message":"Patch Set 3:\n\n\u003e Regarding using kwargs, I would really prefer to avoid them. For\n \u003e HttpClient in this patch I would just use normal arguments. For\n \u003e cases when we have 10+ arguments it seems to be a usual Python\n \u003e practice to use kwargs and have a docstring that describes all\n \u003e parameters. This docstring also helps with code assist also. This\n \u003e approach seems to be reasonable because in our Client class we had\n \u003e a huge number of arguments and it was a mess, too. So I think\n \u003e Winson made a good change here.\n \u003e \n \u003e No need to revert the whole change, let\u0027s improve only places that\n \u003e we still don\u0027t like.\n\nI find it hard to buy into this. Here\u0027s why:\n\n1. - if a function has many parameters, then it has many parameters. Why hide those? Why make it impossible for the interpreter to have a slight chance of catching a typo or bad argument? Why depend on a free form docstring, a string that can contain anything? Why is it better to describe the same \u0027default value\u0027 logic in the body of the function than declaring the same in the signature? Documentation is actually just necessary garbage that is needed to enable us, humans, to understand a piece of code better. \n2. - if a function has many parameters, then refactor it into several functions. Or else, go back to #1.\n\nIf kwargs is used to slim down the number of parameters, then it is a very questionable practice. As I understand, kwargs has probably one real use case:\n\n    When the parameters are not known in advance, e.g. an annotation, or use it to pass\n    through parameters without using them (or only to check for patterns)\n\nAs soon as kwargs are used, automated refactoring becomes impossible, the code has to be checked for parameters and their usage, a docstring needs to be maintained to be in sync with reality and the rate of WTF significantly rises due to ambiguity.\n\nSorry for the rant. I just would like all of us to think about these aspects, too, when writing code or accepting changes.","accounts_in_message":[],"_revision_number":3},{"id":"67e36074b12c402f3d5df7570fc13270dfe86743","author":{"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"},"date":"2016-09-27 09:20:23.000000000","message":"Patch Set 3:\n\n\u003e Instead of just reverting the change. Is there a recommendation on\n \u003e how to implement?\n\nInstead of making everything kwargs, use it just in places where the parameters are handled as opaque values. Even in that case, I\u0027d much rather go for a DTO-like object as it would clearly show the interface. Wherever one would write req.get(\"sth\", defaultValue), I see no reason for not declaring this as a simple optional parameter.","accounts_in_message":[],"_revision_number":3},{"id":"97f4db7c89c06b29bcb53ad8926c861c2d0d29f9","author":{"_account_id":8731,"name":"Renat Akhmerov","email":"renat.akhmerov@gmail.com","username":"rakhmerov"},"date":"2016-09-27 12:44:58.000000000","message":"Patch Set 3:\n\nOk. Andras, I accept your reasoning. Just didn\u0027t assume that this would make somebody upset this much. I also like clear interfaces (because I come from Java too) and agree that we need to care about it.\n\nAlthough, like I said, in Python and OpenStack specifically it\u0027s a normal practice to use kwargs in case of many parameters, not only when we don\u0027t know what they can be. So please let\u0027s please take this fact into account too, discuss again what the best way is and move forward. Ok?\n\nWinson, please also reply what you think.\n\nThanks","accounts_in_message":[],"_revision_number":3},{"id":"73de6c344947c80edbc2e72e27bf300840fb6d8c","author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"date":"2016-09-27 20:17:43.000000000","message":"Patch Set 3:\n\nSo I suggest that we revisit the goal of this patch. The goal is to abstract authentication. Each authentication mechanism will have different requirements for input args. The concrete plugin can be implemented and installed outside of this project (beauty of stevedore).  So it doesn\u0027t makes sense to hard code the input args just for keystone and keycloak in this case.  The use of kwargs is to allow for this flexibility for other auth plugins.  I can understand if there\u0027s a concrete security concerns with using kwargs.","accounts_in_message":[],"_revision_number":3},{"id":"a576c9171e5810b30ecb0c06db4929185c35b037","author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"date":"2016-09-29 09:55:36.000000000","message":"Patch Set 3:\n\nI am also very late to this discussion, but I want to add that this change is a regression in Mistral client. Previously *args are accepted (implicitly). args can be passed to kwargs, unless **kwargs is used, then it wont work.\n\nThe problem can be demonstrated here:\n\n\u003e \u003e\u003e\u003e def fn(a, b\u003d2):\n\u003e ...     print a, b\n\u003e ... \n\u003e \u003e\u003e\u003e def fn2(a, **kwargs):\n\u003e ...     print a, kwargs.get(\u0027b\u0027)\n\u003e ... \n\u003e \u003e\u003e\u003e fn(1, b\u003d2)\n\u003e 1 2\n\u003e \u003e\u003e\u003e fn2(1, b\u003d2)\n\u003e 1 2\n\u003e \u003e\u003e\u003e fn(1, 2)\n\u003e 1 2\n\u003e \u003e\u003e\u003e fn2(1, 2)\n\u003e Traceback (most recent call last):\n\u003e   File \"\u003cstdin\u003e\", line 1, in \u003cmodule\u003e\n\u003e TypeError: fn2() takes exactly 1 argument (2 given)","accounts_in_message":[],"_revision_number":3},{"id":"4adedfc4c7d4f79993a81ef3623f041ed8a3bc87","author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"date":"2016-09-29 09:56:11.000000000","message":"Patch Set 3:\n\nOh, and just to add a re-world use case of this problem. This change caused a breakage in the TripleO CI which was passing in args.","accounts_in_message":[],"_revision_number":3},{"id":"3383fd75b7c74e96163d253f14ab86723aa8ad44","author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"date":"2016-09-29 23:26:36.000000000","message":"Patch Set 3:\n\nEven if we go back to specifying optional args in the function, I would suggest fixing TripleO CI to explicitly pass args in this case.  i.e. fn(1, b\u003d2).  What if we decide to change the ordering or the input args?","accounts_in_message":[],"_revision_number":3},{"id":"a6285ae2628a0b4df5071e072ccd76d37caaebdc","author":{"_account_id":21970,"name":"Andras Kovi","email":"akovi@nokia.com","username":"andras.kovi"},"date":"2016-09-30 09:01:16.000000000","message":"Patch Set 3:\n\n\u003e Even if we go back to specifying optional args in the function, I\n \u003e would suggest fixing TripleO CI to explicitly pass args in this\n \u003e case.  i.e. fn(1, b\u003d2).  What if we decide to change the ordering\n \u003e or the input args?\n\nDoesn\u0027t this mean that we should handle the client as a public API and make sure it is backward compatible? What is the practice in Openstack for these cases? I guess, Mistral and many other project would be in trouble if Heat decided to change the way they initialize their client.","accounts_in_message":[],"_revision_number":3},{"id":"97b931402013c3115160d2a26d9b7c0a7ca35c8a","author":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"date":"2016-10-04 22:57:42.000000000","message":"Patch Set 3:\n\nIMHO, it\u0027s the way that the user is consuming the client that is making it not backward compatible. If the user is explicit in using the optional args, fn(1, b\u003d2) vs fn(1, 2), then it doesn\u0027t matter how we ordered the input args and whether we are using args vs. kwargs.","accounts_in_message":[],"_revision_number":3},{"id":"6755b98e9e37e63ecc48b4295437e8a722ed6176","author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"date":"2016-10-05 06:45:18.000000000","message":"Patch Set 3:\n\n\u003e Even if we go back to specifying optional args in the function, I would suggest fixing TripleO CI to explicitly pass args in this case.  i.e. fn(1, b\u003d2).  What if we decide to change the ordering or the input args?\n\nI have updated TripleO. However, you should not change the order. That would also be backwards incompatible.","accounts_in_message":[],"_revision_number":3},{"id":"a2c522a7841553a83d3be15233c60c639bc79962","author":{"_account_id":9712,"name":"Dougal Matthews","email":"dougal.matthews@canonical.com","username":"dougal"},"date":"2016-10-05 06:48:49.000000000","message":"Patch Set 3:\n\n\u003e IMHO, it\u0027s the way that the user is consuming the client that is making it not backward compatible. If the user is explicit in using the optional args, fn(1, b\u003d2) vs fn(1, 2), then it doesn\u0027t matter how we ordered the input args and whether we are using args vs. kwargs.\n\nI don\u0027t really agree with this logic. Just because it was used in a way that wasn\u0027t totally expected doesn\u0027t mean the consumer was at fault.","accounts_in_message":[],"_revision_number":3}],"current_revision_number":3,"current_revision":"55d55bcdf1240598dd570917531f5100cedfaa73","revisions":{"2d6eb48ea41529492faf568894871d958893c8ee":{"kind":"REWORK","_number":1,"created":"2016-09-16 01:48:19.000000000","uploader":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"ref":"refs/changes/34/371234/1","fetch":{"anonymous http":{"url":"https://review.opendev.org/openstack/python-mistralclient","ref":"refs/changes/34/371234/1","commands":{"Checkout":"git fetch https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/1 \u0026\u0026 git checkout FETCH_HEAD","Cherry Pick":"git fetch https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/1 \u0026\u0026 git cherry-pick FETCH_HEAD","Format Patch":"git fetch https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/1 \u0026\u0026 git format-patch -1 --stdout FETCH_HEAD","Pull":"git pull https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/1"}}},"commit":{"parents":[{"commit":"780debff60e96bedd0502fee396ab1efb836627d","subject":"Merge \"Use requests-mock for testing\"","web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-mistralclient/commit/780debff60e96bedd0502fee396ab1efb836627d"}]}],"author":{"name":"Winson Chan","email":"wcchan@stackstorm.com","date":"2016-09-16 01:28:10.000000000","tz":0},"committer":{"name":"Winson Chan","email":"wcchan@stackstorm.com","date":"2016-09-16 01:28:10.000000000","tz":0},"subject":"Abstract authentication function","message":"Abstract authentication function\n\nAbstract authentication function so plugins for other authentication\nbackends can be implemented in cases where keystone is not used. Currently,\nmistral is hard coded to support keystone and keycloak.\n\nChange-Id: If6ff35e91c3d35c2741332c7e739bb92b1234c54\nImplements: blueprint mistral-abstract-auth\n","web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-mistralclient/commit/2d6eb48ea41529492faf568894871d958893c8ee"}],"resolve_conflicts_web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-mistralclient/commit/2d6eb48ea41529492faf568894871d958893c8ee"}]},"branch":"refs/heads/master"},"f6402581f810a9e8b85f4493a3d0a3bc37076c42":{"kind":"REWORK","_number":2,"created":"2016-09-21 23:19:40.000000000","uploader":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"ref":"refs/changes/34/371234/2","fetch":{"anonymous http":{"url":"https://review.opendev.org/openstack/python-mistralclient","ref":"refs/changes/34/371234/2","commands":{"Checkout":"git fetch https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/2 \u0026\u0026 git checkout FETCH_HEAD","Cherry Pick":"git fetch https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/2 \u0026\u0026 git cherry-pick FETCH_HEAD","Format Patch":"git fetch https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/2 \u0026\u0026 git format-patch -1 --stdout FETCH_HEAD","Pull":"git pull https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/2"}}},"commit":{"parents":[{"commit":"5438b20c46442e9bacd04a30cf0d81097530c9dc","subject":"Merge \"Pass httpclient to managers\"","web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-mistralclient/commit/5438b20c46442e9bacd04a30cf0d81097530c9dc"}]}],"author":{"name":"Winson Chan","email":"wcchan@stackstorm.com","date":"2016-09-16 01:28:10.000000000","tz":0},"committer":{"name":"Winson Chan","email":"wcchan@stackstorm.com","date":"2016-09-21 23:16:00.000000000","tz":0},"subject":"Abstract authentication function","message":"Abstract authentication function\n\nAbstract authentication function so plugins for other authentication\nbackends can be implemented in cases where keystone is not used. Currently,\nmistral is hard coded to support keystone and keycloak.\n\nChange-Id: If6ff35e91c3d35c2741332c7e739bb92b1234c54\nImplements: blueprint mistral-abstract-auth\n","web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-mistralclient/commit/f6402581f810a9e8b85f4493a3d0a3bc37076c42"}],"resolve_conflicts_web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-mistralclient/commit/f6402581f810a9e8b85f4493a3d0a3bc37076c42"}]},"branch":"refs/heads/master"},"55d55bcdf1240598dd570917531f5100cedfaa73":{"kind":"REWORK","_number":3,"created":"2016-09-22 01:01:24.000000000","uploader":{"_account_id":5558,"name":"Winson Chan","email":"wcchan@stackstorm.com","username":"winson-c-chan"},"ref":"refs/changes/34/371234/3","fetch":{"anonymous http":{"url":"https://review.opendev.org/openstack/python-mistralclient","ref":"refs/changes/34/371234/3","commands":{"Checkout":"git fetch https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/3 \u0026\u0026 git checkout FETCH_HEAD","Cherry Pick":"git fetch https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/3 \u0026\u0026 git cherry-pick FETCH_HEAD","Format Patch":"git fetch https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/3 \u0026\u0026 git format-patch -1 --stdout FETCH_HEAD","Pull":"git pull https://review.opendev.org/openstack/python-mistralclient refs/changes/34/371234/3"}}},"commit":{"parents":[{"commit":"5438b20c46442e9bacd04a30cf0d81097530c9dc","subject":"Merge \"Pass httpclient to managers\"","web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-mistralclient/commit/5438b20c46442e9bacd04a30cf0d81097530c9dc"}]}],"author":{"name":"Winson Chan","email":"wcchan@stackstorm.com","date":"2016-09-16 01:28:10.000000000","tz":0},"committer":{"name":"Winson Chan","email":"wcchan@stackstorm.com","date":"2016-09-22 01:00:37.000000000","tz":0},"subject":"Abstract authentication function","message":"Abstract authentication function\n\nAbstract authentication function so plugins for other authentication\nbackends can be implemented in cases where keystone is not used. Currently,\nmistral is hard coded to support keystone and keycloak.\n\nChange-Id: If6ff35e91c3d35c2741332c7e739bb92b1234c54\nImplements: blueprint mistral-abstract-auth\n","web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-mistralclient/commit/55d55bcdf1240598dd570917531f5100cedfaa73"}],"resolve_conflicts_web_links":[{"name":"gitea","tooltip":"Open in GitWeb","url":"https://opendev.org/openstack/python-mistralclient/commit/55d55bcdf1240598dd570917531f5100cedfaa73"}]},"branch":"refs/heads/master"}},"requirements":[],"submit_records":[],"submit_requirements":[]}
