)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"aea7f9c846cc5215b8ec323d6dcfb322a5727074","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"cbd96182_510b7aa0","updated":"2022-11-23 16:48:38.000000000","message":"Hi Stephen, few comments inline before i abandon this patch.","commit_id":"bd741b96367aade8ae3509d06b229b28e6714eb4"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"d52937289e28af641c6b218c9043b3c1043c0404","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fc6ba126_6d0323e6","updated":"2022-11-23 13:33:33.000000000","message":"I appreciate what you\u0027re trying to do here but I\u0027m afraid we can\u0027t and won\u0027t do this. There are a few reasons for this but the most pressing one is that it\u0027s a *huge* breaking change that will break multiple users\u0027 scripts. Consider a script that did this, for example.\n\n  openstack volume create --snapshot my_snapshot 123\n\nPreviously this would have created a snapshot with name\u003d123 and size\u003d$my_snapshot.size. With this change, this will create a snapshot with size\u003d123 and name\u003d\u003crandom UUID\u003e/null. Likewise, consider a command like this:\n\n  openstack volume create --size 123 foo\n\nThis would now be an invalid command since \u0027--size\u0027 is no longer a valid option and \u0027name\u0027 is no longer a valid positional argument.\n\nBreaking changes aside, there\u0027s a bigger philosophical point here. You indicate that we expect there to be a smooth transition from cinderclient to openstackclient. This is an important goal, yes, but there is a far *more* important goal: namely, uniformity between OSC commands. This is explicitly called out in the summary from the README:\n\n  OpenStackClient (aka OSC) is a command-line client for OpenStack that brings the\n  command set for Compute, Identity, Image, Network, Object Store and Block Storage\n  APIs together in a single shell **with a uniform command structure**.\n\n(emphasis mine).\n\nWhile we should not go out of our way to introduce unnecessary friction, this is a great example of where consistency between OSC commands must trump consistency with the legacy client. OSC \u0027resource create\u0027 commands have a pattern of providing an name or identifier as their positional argument. This change would break this pattern and mean that \u0027volume create\u0027 behaves totally different from commands like \u0027server create\u0027, \u0027network create\u0027, \u0027image create\u0027, \u0027project create\u0027, \u0027user create\u0027, \u0027flavor create\u0027, etc. (and that\u0027s not even thinking about out-of-tree OSC plugins like osc-placement or python-manilaclient...). We can\u0027t and won\u0027t do this. For what it\u0027s worth, other examples of differences exist such as how OSC handles boolean values compared to the legacy clients (--flag/--no-flag vs. --flag\u003d\u003ctrue|false\u003e), its use of the \u0027\u003cresource\u003e set\u0027 and \u0027resource unset\u0027 command to make changes to an existing resource rather than a plethora of \u0027resource \u003caction\u003e\u0027 commands, which only exist where they make sense (I can go into this in more detail if you\u0027d like).\n\nI understand that you care about your users and would like them to be able to migrate to OSC as easily as possible. To make this easier, I would suggest looking at ways to fix _cinderclient_ so that the OSC-style of pattern is possible. Alternatively, document the heck out of this. Nova migrated all of its in-tree documentation to use OSC some time ago and to the best of my knowledge, most of Red Hat\u0027s downstream docs have also been updated now (I can\u0027t speak for other vendors). Before this happened though, we gave examples of OSC usage _alongside_ novaclient usage and this was helpful to compare the two. Either of these options would help your users without diminishing one of the biggest value-adds of OSC: its uniformity.\n\nHope that all makes sense. I\u0027m happy to discuss this in greater detail if you wish.","commit_id":"bd741b96367aade8ae3509d06b229b28e6714eb4"},{"author":{"_account_id":15334,"name":"Stephen Finucane","display_name":"stephenfin","email":"stephenfin@redhat.com","username":"sfinucan"},"change_message_id":"9b88262b06c4f472fbea2b63b35ffe5ba0bc4c2f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d427a8b2_85741bed","in_reply_to":"d21492c1_69bb1300","updated":"2022-11-24 12:52:21.000000000","message":"\u003e \u003e I appreciate what you\u0027re trying to do here but I\u0027m afraid we can\u0027t and won\u0027t do this. There are a few reasons for this but the most pressing one is that it\u0027s a *huge* breaking change that will break multiple users\u0027 scripts. \n\u003e \n\u003e I understand the concern but doesn\u0027t any major change causes this issue? We recently removed cinder V2 so any script doing API requests to it will fail and they need to update it.\n\nYes and that was a bad thing since it turns out there are quite a few deployments running older cinder versions 😄 Talk to gtema if you want to know more. Long story short, we\u0027re now working to migrate all cinder-related commands from cinderclient to SDK since that continues to support v2.\n\n\u003e \u003e Consider a script that did this, for example.\n\u003e \u003e \n\u003e \u003e   openstack volume create --snapshot my_snapshot 123\n\u003e \n\u003e I highly doubt someone in a production environment would be running a script setting names as numbers.\n\u003e For others, that are using good identifiers as names, they will surely contain alpha characters and will fail here so they will know it\u0027s time to update the script.\n\u003e In any case, we won\u0027t end up creating volumes with a crazy amount of size.\n\nI agree, but like cinder and mostly everyone else, we don\u0027t break stuff like this. Since this is breaking change, you need at least 6 months or 1 release worth of a deprecation cycle, no different to most other OpenStack projects. This is all a moot discussion given my consistency comments below.\n\n\u003e \u003e Previously this would have created a snapshot with name\u003d123 and size\u003d$my_snapshot.size. With this change, this will create a snapshot with size\u003d123 and name\u003d\u003crandom UUID\u003e/null. Likewise, consider a command like this:\n\u003e \u003e \n\u003e \u003e   openstack volume create --size 123 foo\n\u003e \u003e \n\u003e \u003e This would now be an invalid command since \u0027--size\u0027 is no longer a valid option and \u0027name\u0027 is no longer a valid positional argument.\n\u003e \n\u003e Yes, and that is the purpose of this change. since their scripts will fail, they can update it. They won\u0027t end up in a case where this will succeed in a weird way that is undetectable and will cause issues later.\n\u003e \n\u003e \u003e \n\u003e \u003e Breaking changes aside, there\u0027s a bigger philosophical point here. You indicate that we expect there to be a smooth transition from cinderclient to openstackclient. This is an important goal, yes, but there is a far *more* important goal: namely, uniformity between OSC commands. This is explicitly called out in the summary from the README:\n\u003e \u003e \n\u003e \u003e   OpenStackClient (aka OSC) is a command-line client for OpenStack that brings the\n\u003e \u003e   command set for Compute, Identity, Image, Network, Object Store and Block Storage\n\u003e \n\u003e I\u0027m in favor of consistency but at the same time, we are comparing cinder to other projects who followed similar structure to OSC in their project specific clients. If we take a look at the project specific clients,\n\u003e \n\u003e 1) Nova: it was always mandatory to provide a name in ``nova boot`` command so they don\u0027t see any change\n\u003e 2) Glance: glance has no parameter as mandatory in glance client so they also don\u0027t seem to bother if name becomes positional.\n\u003e 3) Keystone: keystone client also had the requirement to pass name argument\n\u003e keystone tenant-create --name \u003ctenant-name\u003e\n\nThis is kind of my point, the APIs of each service behaves differently and unless you\u0027re an OpenStack power user/contributor, this discrepancy will annoying at best and downright confusing at worst. OSC\u0027s primary goal is to hide or smooth over these differences where it makes sense and as such we enforce the pattern of \u0027resource create \u003chuman-readable identifer\u003e\u0027 I mentioned in my email.\n\nThis being said, we actually have made an exception for cinder, in that \u0027\u003chuman-readable identifer\u003e\u0027 is now optional. tbh, I\u0027ve no idea why this isn\u0027t good enough. Not insisting on something the API doesn\u0027t need is a fair argument but consistency with the legacy client isn\u0027t, especially when it conflicts with OSC\u0027s own goals. It\u0027s not like users can simply replace \u0027cinder create\u0027 with \u0027openstack volume create\u0027 and expect everything to work, right? We have different name for most other options (\u0027--backup\u0027 vs. \u0027--backup-id\u0027, \u0027--property\u0027 vs \u0027--metadata\u0027, etc.). Why is a user capable of changing \u0027--backup-id \u003cbackup\u003e\u0027 to \u0027--backup-id \u003cbackup\u003e\u0027 yet they can\u0027t change \u0027--name \u003cname\u003e \u003csize\u003e\u0027 to \u0027--size \u003csize\u003e \u003cname\u003e\u0027?\n\n\u003e \u003e   APIs together in a single shell **with a uniform command structure**.\n\u003e \u003e \n\u003e \u003e (emphasis mine).\n\u003e \u003e \n\u003e \u003e While we should not go out of our way to introduce unnecessary friction, this is a great example of where consistency between OSC commands must trump consistency with the legacy client. OSC \u0027resource create\u0027 commands have a pattern of providing an name or identifier as their positional argument. This change would break this pattern and mean that \u0027volume create\u0027 behaves totally different from commands like \u0027server create\u0027, \u0027network create\u0027, \u0027image create\u0027, \u0027project create\u0027, \u0027user create\u0027, \u0027flavor create\u0027, etc. (and that\u0027s not even thinking about out-of-tree OSC plugins like osc-placement or python-manilaclient...). We can\u0027t and won\u0027t do this. For what it\u0027s worth, other examples of differences exist such as how OSC handles boolean values compared to the legacy clients (--flag/--no-flag vs. --flag\u003d\u003ctrue|false\u003e), its use of the \u0027\u003cresource\u003e set\u0027 and \u0027resource unset\u0027 command to make changes to an existing resource rather than a plethora of \u0027resource \u003caction\u003e\u0027 commands, which only exist where they make sense (I can go into this in more detail if you\u0027d like).\n\u003e \u003e \n\u003e \u003e I understand that you care about your users and would like them to be able to migrate to OSC as easily as possible. To make this easier, I would suggest looking at ways to fix _cinderclient_ so that the OSC-style of pattern is possible. Alternatively, document the heck out of this. Nova migrated all of its in-tree documentation to use OSC some time ago and to the best of my knowledge, most of Red Hat\u0027s downstream docs have also been updated now (I can\u0027t speak for other vendors). Before this happened though, we gave examples of OSC usage _alongside_ novaclient usage and this was helpful to compare the two. Either of these options would help your users without diminishing one of the biggest value-adds of OSC: its uniformity.\n\u003e \u003e \n\u003e \u003e Hope that all makes sense. I\u0027m happy to discuss this in greater detail if you wish.\n\u003e \n\u003e For cinder\u0027s case (and other cinder cores can agree/disagree with me), size is far more important than name and should have been in that order since the beginning\n\nIt may be more important, but that doesn\u0027t mean it has to be a positional argument. Cinder isn\u0027t unique here. Consider compute flavors. A flavor\u0027s RAM, CPU and disk properties are every bit as important as the name (you can\u0027t create a flavor without any of these), yet only \u0027\u003cname\u003e\u0027 is a positional argument in OSC and the rest are required options. This allows us to be consistent with the other \u0027resource create\u0027 commands. It\u0027s also a lot nicer to use. These commands do the same thing. Which is more obvious?\n\n  nova flavor-create foo 1000 4 2 1\n  openstack flavor create --id 1000 --ram 4 --disk 2 --vcpus 1 foo\n\n\u003e but we can\u0027t change the past now and due to the nature of our projects being backward compatible, that is another hurdle to make this change not possible.\n\nThis backwards compatibility also applies to OSC, as noted above.\n\n\u003e In conclusion, I still prefer what I\u0027ve proposed here but the amount of effort we are putting here seems to outweigh the benefit which end users will get from this change so I will abandon it.\n\u003e Thanks for the discussion.\n\nNo worries. Hopefully you can at least grasp what we\u0027re going for here, even if you don\u0027t entirely agree. It\u0027s coming from a good place 😊","commit_id":"bd741b96367aade8ae3509d06b229b28e6714eb4"},{"author":{"_account_id":27615,"name":"Rajat Dhasmana","email":"rajatdhasmana@gmail.com","username":"whoami-rajat"},"change_message_id":"aea7f9c846cc5215b8ec323d6dcfb322a5727074","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"d21492c1_69bb1300","in_reply_to":"fc6ba126_6d0323e6","updated":"2022-11-23 16:48:38.000000000","message":"\u003e I appreciate what you\u0027re trying to do here but I\u0027m afraid we can\u0027t and won\u0027t do this. There are a few reasons for this but the most pressing one is that it\u0027s a *huge* breaking change that will break multiple users\u0027 scripts. \n\nI understand the concern but doesn\u0027t any major change causes this issue? We recently removed cinder V2 so any script doing API requests to it will fail and they need to update it.\n\n\u003e Consider a script that did this, for example.\n\u003e \n\u003e   openstack volume create --snapshot my_snapshot 123\n\u003e \n\nI highly doubt someone in a production environment would be running a script setting names as numbers.\nFor others, that are using good identifiers as names, they will surely contain alpha characters and will fail here so they will know it\u0027s time to update the script.\nIn any case, we won\u0027t end up creating volumes with a crazy amount of size.\n\n\u003e Previously this would have created a snapshot with name\u003d123 and size\u003d$my_snapshot.size. With this change, this will create a snapshot with size\u003d123 and name\u003d\u003crandom UUID\u003e/null. Likewise, consider a command like this:\n\u003e \n\u003e   openstack volume create --size 123 foo\n\u003e \n\u003e This would now be an invalid command since \u0027--size\u0027 is no longer a valid option and \u0027name\u0027 is no longer a valid positional argument.\n\nYes, and that is the purpose of this change. since their scripts will fail, they can update it. They won\u0027t end up in a case where this will succeed in a weird way that is undetectable and will cause issues later.\n\n\u003e \n\u003e Breaking changes aside, there\u0027s a bigger philosophical point here. You indicate that we expect there to be a smooth transition from cinderclient to openstackclient. This is an important goal, yes, but there is a far *more* important goal: namely, uniformity between OSC commands. This is explicitly called out in the summary from the README:\n\u003e \n\u003e   OpenStackClient (aka OSC) is a command-line client for OpenStack that brings the\n\u003e   command set for Compute, Identity, Image, Network, Object Store and Block Storage\n\nI\u0027m in favor of consistency but at the same time, we are comparing cinder to other projects who followed similar structure to OSC in their project specific clients. If we take a look at the project specific clients,\n\n1) Nova: it was always mandatory to provide a name in ``nova boot`` command so they don\u0027t see any change\n2) Glance: glance has no parameter as mandatory in glance client so they also don\u0027t seem to bother if name becomes positional.\n3) Keystone: keystone client also had the requirement to pass name argument\nkeystone tenant-create --name \u003ctenant-name\u003e\n\n\u003e   APIs together in a single shell **with a uniform command structure**.\n\u003e \n\u003e (emphasis mine).\n\u003e \n\u003e While we should not go out of our way to introduce unnecessary friction, this is a great example of where consistency between OSC commands must trump consistency with the legacy client. OSC \u0027resource create\u0027 commands have a pattern of providing an name or identifier as their positional argument. This change would break this pattern and mean that \u0027volume create\u0027 behaves totally different from commands like \u0027server create\u0027, \u0027network create\u0027, \u0027image create\u0027, \u0027project create\u0027, \u0027user create\u0027, \u0027flavor create\u0027, etc. (and that\u0027s not even thinking about out-of-tree OSC plugins like osc-placement or python-manilaclient...). We can\u0027t and won\u0027t do this. For what it\u0027s worth, other examples of differences exist such as how OSC handles boolean values compared to the legacy clients (--flag/--no-flag vs. --flag\u003d\u003ctrue|false\u003e), its use of the \u0027\u003cresource\u003e set\u0027 and \u0027resource unset\u0027 command to make changes to an existing resource rather than a plethora of \u0027resource \u003caction\u003e\u0027 commands, which only exist where they make sense (I can go into this in more detail if you\u0027d like).\n\u003e \n\u003e I understand that you care about your users and would like them to be able to migrate to OSC as easily as possible. To make this easier, I would suggest looking at ways to fix _cinderclient_ so that the OSC-style of pattern is possible. Alternatively, document the heck out of this. Nova migrated all of its in-tree documentation to use OSC some time ago and to the best of my knowledge, most of Red Hat\u0027s downstream docs have also been updated now (I can\u0027t speak for other vendors). Before this happened though, we gave examples of OSC usage _alongside_ novaclient usage and this was helpful to compare the two. Either of these options would help your users without diminishing one of the biggest value-adds of OSC: its uniformity.\n\u003e \n\u003e Hope that all makes sense. I\u0027m happy to discuss this in greater detail if you wish.\n\nFor cinder\u0027s case (and other cinder cores can agree/disagree with me), size is far more important than name and should have been in that order since the beginning but we can\u0027t change the past now and due to the nature of our projects being backward compatible, that is another hurdle to make this change not possible.\nIn conclusion, I still prefer what I\u0027ve proposed here but the amount of effort we are putting here seems to outweigh the benefit which end users will get from this change so I will abandon it.\nThanks for the discussion.","commit_id":"bd741b96367aade8ae3509d06b229b28e6714eb4"}]}