)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"2d4f729626c7f4e61a808525e8e5aa93a233c27d","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2d86e7b5_d7dccc30","updated":"2022-04-29 13:40:15.000000000","message":"Like I said on the other patch, this reveals a deeper problem with the structure of glance tox.ini, not just the \u0027docs\u0027 target.  This patch will fix the docs build, but it does not solve the actual problem.\n\nSee https://opendev.org/openstack/cinder/commit/5ad7fe92690b29a05c6284aa52e01913b5b7cf76 for what I think needs to be done to fix the actual problem.","commit_id":"a46a43f063c9c6d5d60b7e53cbbe128bb7228997"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"216de7935e54d455ea4af7e4f19710b06f0d0c76","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2d28e425_cc105dd8","updated":"2022-04-29 13:22:30.000000000","message":"Will leave approval to rosmaita as he has some questions about this.","commit_id":"a46a43f063c9c6d5d60b7e53cbbe128bb7228997"},{"author":{"_account_id":17685,"name":"Elod Illes","email":"elod.illes@est.tech","username":"elod.illes"},"change_message_id":"89da55d06601482bd3c9668ed896b6b478e5f775","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"fb891727_2d7b47f6","in_reply_to":"2d86e7b5_d7dccc30","updated":"2022-04-29 14:10:28.000000000","message":"There is some history in this :) In past almost every project used install_command and upper constraints in it. And some places there were extra constraints file linked (like lower-constraints.txt; yeah i know, it\u0027s past now) in deps, but pip only consider the 1st constraints file (which caused multiple problems). Then most of the projects (?) removed install_command and only used deps for list there everything (constraints, requirements).\n\nFor now: it does not matter where we use the constraints (afaik) it could be either in install_command or deps, but not in both. In that sense you are right if I\u0027m not mistaken that install_command is used in develop-inst phase as well, so then there\u0027s no need to explicitly add requirements.txt in any tox target. The only question is: is there other reason why it is not fortunate to define install_command? (I really remember it was discouraged to use it, but don\u0027t know other details... but maybe i\u0027m wrong)\n\nNevertheless, I\u0027m OK with both ways (both solves the problem), so let me know and I update the patch accordingly. o:)","commit_id":"a46a43f063c9c6d5d60b7e53cbbe128bb7228997"},{"author":{"_account_id":17685,"name":"Elod Illes","email":"elod.illes@est.tech","username":"elod.illes"},"change_message_id":"58058a005e60b7e2ffecc87c549dcb397db2b1a9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"76266e46_e616cfd5","in_reply_to":"6654b322_7a797766","updated":"2022-05-05 15:24:19.000000000","message":"Thanks @Brian for the answer. I did a quick search and it seems that install_command was recommended only due to practical reasons [1]:\n\n\"Some projects have upper-constraints.txt in install_command but\nexplicitly override it in lower-constratints.txt this works and wont be\nchanged at this point but ideally all projects will use the same format\nto reduce the impact in switching between projects.\"\n\nI don\u0027t have hard feeling for the solution, so let this issue just be fixed for now. I\u0027ve updated the patch according your suggestion. Thanks again for the review!\n\n[1] http://lists.openstack.org/pipermail/openstack-discuss/2019-May/006478.html","commit_id":"a46a43f063c9c6d5d60b7e53cbbe128bb7228997"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"4bcc34f14d1a2b82c1c4fb50d6d3f0d042f829c7","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"6654b322_7a797766","in_reply_to":"fb891727_2d7b47f6","updated":"2022-05-05 14:34:19.000000000","message":"My memory of the install_command removal was that we were discouraged from using it when the lower-constraints job was introduced (because obviously, it would make that job not work correctly).  However, in cinder or cinderlib, after wasting many developer-hours, we finally realized that what we really needed to do was keep the -c upper-constraints.txt in the install_command for the base testenv, and override it to be the non-constrained install_command for the special-purpose lower-constraints job (which most projects have removed at this point).\n\nAnyway ... the problem was really hard to track down, and we only found it because there was a new release of some dependency that broke one of our CI jobs, even though that version was explicitly excluded by upper-constraints.\n\nBTW, I\u0027m pretty sure the constraints can be both in deps and in the install command; the key thing is that if the constraints are *not* in the install command, they aren\u0027t used in the usedevelop install.  We were discouraged from defining them in 2 places just because that\u0027s always a problem when updating things (you might miss one).\n\nIn summary, I\u0027m not -2ing this, but I\u0027m not +2 on it either.","commit_id":"a46a43f063c9c6d5d60b7e53cbbe128bb7228997"},{"author":{"_account_id":5314,"name":"Brian Rosmaita","email":"rosmaita.fossdev@gmail.com","username":"brian-rosmaita"},"change_message_id":"3c15ec07cac927d608c0117294023393ec5dd603","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"8a6459cb_719c92c6","updated":"2022-05-06 13:37:47.000000000","message":"@Elod: thanks for tracking down the history on this!  Good to know that there\u0027s no technical reason against this change.\n\nLGTM and Zuul is happy, so let\u0027s merge it!","commit_id":"36aedba20e998953333225a9788c084d2e6be6ae"},{"author":{"_account_id":9303,"name":"Abhishek Kekane","email":"akekane@redhat.com","username":"abhishekkekane"},"change_message_id":"d6968d7e645a4411237bdf3719c06259149bb4c2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"41f1a7c2_977851f6","updated":"2022-05-06 05:44:51.000000000","message":"Thank you!","commit_id":"36aedba20e998953333225a9788c084d2e6be6ae"}]}
