)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"0726284323d0828c153b6da1c0e2924736916987","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e415a643_fb36b936","updated":"2025-06-20 18:09:37.000000000","message":"We can abandon this one in favor of:\nhttps://review.opendev.org/c/openstack/watcher-tempest-plugin/+/952884","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d5ee8a8217d514c19d654a9acd52898073eb047c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"da6c02c3_880eec90","updated":"2025-06-19 00:17:00.000000000","message":"im conflicted because there are sitll other issues with this patch.","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"7bc0a6be08231581ad0496d214ec38237af5a284","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"1241ee69_f6a42299","updated":"2025-06-16 11:53:40.000000000","message":"lgtm, thanks","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"5428099da63ac5d56b9b06a6995a1b3f62227cf5","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"fef40b24_cad92c45","updated":"2025-06-16 13:00:21.000000000","message":"looks correct","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2468976864d354e2d9d18570900fa2855689426c","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"e3c409bb_029afcb9","updated":"2025-06-16 14:15:29.000000000","message":"this change does not make sense to me.\n\nif you need to create an single instnace on a specific host\nyou shoudl do that in the test without reusing this fucntion.","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"c28ca3d5800f65ffd05e2529c429f58880604be6","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"dd198078_7f665d64","in_reply_to":"32d177d8_65733418","updated":"2025-06-18 06:29:00.000000000","message":"I think i mixed the reviews comments from here https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/949722/44/watcher_tempest_plugin/tests/scenario/test_execute_strategies.py#142","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"037b83d8bb0710db27c653dcdf93a57782586343","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"32d177d8_65733418","in_reply_to":"5207453f_a1eaf1b6","updated":"2025-06-18 04:28:38.000000000","message":"Thank you Sean and David for the interesting discussion.\n+1 to the idea of creating instance on specific host and I also agree with Sean\u0027s comment around anti pattern.\n\nCan we do something like this?\n1. Can we create a create_server wrapper function which will call tempest _create_server method which will take parameters as no_of_instance\u003d1 and compute_nodes to none or list.\n This method will create one instance on any host or multiple instances on specific host. We can also pass run_command and metric params to that also?\n \n This will help to keep create server logic at one place.\n \n2. we will re use the new create server wrapper in _create_instances_per_host_with_statistic to keep this method to create one instance per host\n\n3. We can add another function to create multiple instances on specific host by reusing the wrapper function with in that.\n\nI think this will solve the above concern.\nSean, David what do you say about above approach?","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0c06c987a7ad7640656dc8abe68d1ee331009974","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":2,"id":"5207453f_a1eaf1b6","in_reply_to":"b55b5bab_5c024599","updated":"2025-06-17 10:06:10.000000000","message":"right nad keeping all the server creation logic in that function is explicitly not something we should be doing.\n\nThe Tempest convention is to create a server as needed in the test function\nand make it very clear how they are created.\n\nCentralising the creation in this function is the thing I said in our meeting that I didn\u0027t want to see done.\n\nits very much an anti pattern.\n\nHaving created server functions is fine the anti-pattern here is having a function to create multiple servers by default and coupling the creation with the metrics injection. \n\nthis funciton shoudl really only be used for the workload consolidation stragey and even then keepign it at all is questionable.\n\nthis was on the top of my list to remove when i first looked at the plugin but we did not have time to do that then. that why i warnd that i didnt want to see use get mroe dependent on that.\n\nwhen we discussted this change inially i was expecting you to modify the function sot that it ineternally specifed which host each vm would be spanwed on. not that you would allow choosing which host as this fucntion is only for the case wehre you want exactly 1 vm on each host.","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":28647,"name":"David","display_name":"morenod","email":"dsanzmor@redhat.com","username":"morenod"},"change_message_id":"6801f00ca6ba8f1691fbd4d8710d94075a26cbb2","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":2,"id":"b55b5bab_5c024599","in_reply_to":"e3c409bb_029afcb9","updated":"2025-06-17 09:05:18.000000000","message":"reusing the function allow us to manage the creation of servers on a single point, avoiding duplicate code and centralizing all the instance-creation-topics on it.\n\nthis change keeps the current behavior of the function as default, everything that was working before, will keep working, and introduce the ability of selecting the host, with a minimal change on the code","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"}],"watcher_tempest_plugin/tests/scenario/base.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"7bc0a6be08231581ad0496d214ec38237af5a284","unresolved":true,"context_lines":[{"line_number":264,"context_line":"                                                  run_command\u003dNone,"},{"line_number":265,"context_line":"                                                  inject\u003dTrue,"},{"line_number":266,"context_line":"                                                  num_instances\u003d1,"},{"line_number":267,"context_line":"                                                  compute_node\u003dNone):"},{"line_number":268,"context_line":"        \"\"\"Create instance per compute node and make instance statistic"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        This goes up to the min_compute_nodes threshold so that things don\u0027t"}],"source_content_type":"text/x-python","patch_set":2,"id":"7d0b9981_b09318c4","line":267,"range":{"start_line":267,"start_character":50,"end_line":267,"end_character":62},"updated":"2025-06-16 11:53:40.000000000","message":"could also be a list, but that will work for our current tests","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d5ee8a8217d514c19d654a9acd52898073eb047c","unresolved":true,"context_lines":[{"line_number":264,"context_line":"                                                  run_command\u003dNone,"},{"line_number":265,"context_line":"                                                  inject\u003dTrue,"},{"line_number":266,"context_line":"                                                  num_instances\u003d1,"},{"line_number":267,"context_line":"                                                  compute_node\u003dNone):"},{"line_number":268,"context_line":"        \"\"\"Create instance per compute node and make instance statistic"},{"line_number":269,"context_line":""},{"line_number":270,"context_line":"        This goes up to the min_compute_nodes threshold so that things don\u0027t"}],"source_content_type":"text/x-python","patch_set":2,"id":"5062e0b2_9e294343","line":267,"range":{"start_line":267,"start_character":50,"end_line":267,"end_character":62},"in_reply_to":"7d0b9981_b09318c4","updated":"2025-06-19 00:17:00.000000000","message":"I was also thinking that that would be more useful, in my opinion, than invoking this function multiple times with once per host.","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2468976864d354e2d9d18570900fa2855689426c","unresolved":true,"context_lines":[{"line_number":284,"context_line":"        \"\"\""},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        compute_nodes \u003d self.get_compute_nodes_setup()"},{"line_number":287,"context_line":"        if compute_node:"},{"line_number":288,"context_line":"            compute_nodes \u003d [node for node in compute_nodes"},{"line_number":289,"context_line":"                             if node[\u0027host\u0027] \u003d\u003d compute_node]"},{"line_number":290,"context_line":"        instances \u003d self.mgr.servers_client.list_servers("},{"line_number":291,"context_line":"            detail\u003dTrue)[\u0027servers\u0027]"},{"line_number":292,"context_line":"        if instances:"}],"source_content_type":"text/x-python","patch_set":2,"id":"7f0f6875_e28ef40b","line":289,"range":{"start_line":287,"start_character":7,"end_line":289,"end_character":61},"updated":"2025-06-16 14:15:29.000000000","message":"this does not really feel like it fix with this function.\ni was expect this to be a boolean not a specific host.\n\nor for us to just default to alwasy creating the vms and spesifying a host rather then allowing nova to choose.","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":28647,"name":"David","display_name":"morenod","email":"dsanzmor@redhat.com","username":"morenod"},"change_message_id":"6801f00ca6ba8f1691fbd4d8710d94075a26cbb2","unresolved":true,"context_lines":[{"line_number":284,"context_line":"        \"\"\""},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        compute_nodes \u003d self.get_compute_nodes_setup()"},{"line_number":287,"context_line":"        if compute_node:"},{"line_number":288,"context_line":"            compute_nodes \u003d [node for node in compute_nodes"},{"line_number":289,"context_line":"                             if node[\u0027host\u0027] \u003d\u003d compute_node]"},{"line_number":290,"context_line":"        instances \u003d self.mgr.servers_client.list_servers("},{"line_number":291,"context_line":"            detail\u003dTrue)[\u0027servers\u0027]"},{"line_number":292,"context_line":"        if instances:"}],"source_content_type":"text/x-python","patch_set":2,"id":"c674915e_fc872719","line":289,"range":{"start_line":287,"start_character":7,"end_line":289,"end_character":61},"in_reply_to":"7f0f6875_e28ef40b","updated":"2025-06-17 09:05:18.000000000","message":"the idea is to keep the function working as default with the current behavior with a minimal change","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"6e16e2cc13e98b8f8511161f2ce5602d5081e5b1","unresolved":true,"context_lines":[{"line_number":284,"context_line":"        \"\"\""},{"line_number":285,"context_line":""},{"line_number":286,"context_line":"        compute_nodes \u003d self.get_compute_nodes_setup()"},{"line_number":287,"context_line":"        if compute_node:"},{"line_number":288,"context_line":"            compute_nodes \u003d [node for node in compute_nodes"},{"line_number":289,"context_line":"                             if node[\u0027host\u0027] \u003d\u003d compute_node]"},{"line_number":290,"context_line":"        instances \u003d self.mgr.servers_client.list_servers("},{"line_number":291,"context_line":"            detail\u003dTrue)[\u0027servers\u0027]"},{"line_number":292,"context_line":"        if instances:"}],"source_content_type":"text/x-python","patch_set":2,"id":"0cf2ec5f_85c778b3","line":289,"range":{"start_line":287,"start_character":7,"end_line":289,"end_character":61},"in_reply_to":"c674915e_fc872719","updated":"2025-06-17 09:15:59.000000000","message":"I think the idea of being able to specify the host where we want to create the instance is good. Currently, in several cases we need to have multiple instances in one compute node and keep the rest empty. For that, we are currently creating one node per host and then live-migrating all to one host.\n\nInstead of that, which makes jobs longer and more complex, this would allow to do:\n\n_create_instances_per_host_with_statistic(num_instances\u003dX, compute_node\u003d\"computeY\")\n\nwhich is cleaner, faster and less error prone.","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d5ee8a8217d514c19d654a9acd52898073eb047c","unresolved":true,"context_lines":[{"line_number":295,"context_line":"        hypervisors \u003d self.get_hypervisors_setup()"},{"line_number":296,"context_line":"        created_instances \u003d []"},{"line_number":297,"context_line":""},{"line_number":298,"context_line":"        for node in compute_nodes[:CONF.compute.min_compute_nodes]:"},{"line_number":299,"context_line":"            hyp_id \u003d ["},{"line_number":300,"context_line":"                hyp[\u0027id\u0027] for hyp in hypervisors"},{"line_number":301,"context_line":"                if hyp[\u0027hypervisor_hostname\u0027] \u003d\u003d node[\u0027host\u0027]]"}],"source_content_type":"text/x-python","patch_set":2,"id":"a6832e23_74e69549","line":298,"range":{"start_line":298,"start_character":33,"end_line":298,"end_character":65},"updated":"2025-06-19 00:17:00.000000000","message":"As an aside, this is a misuse of the config option\n\nThat option is intended to be used to skip tests that need more than the minimum available\n\n    cfg.IntOpt(\u0027min_compute_nodes\u0027,\n               default\u003d1,\n               help\u003d(\u0027The minimum number of compute nodes expected. This will \u0027\n                     \u0027be utilized by some multinode-specific tests to ensure \u0027\n                     \u0027that requests match the expected size of the cluster \u0027\n                     \u0027you are testing with.\u0027)),\n                     \nUsing that for this logic is a bug.\n\nim tempted to say we shoudl remove this but to do that properly woudl mean  fixing the test to pass in the value of the number of hosts they required.\n\nthat number could come form a new watcher tempest pluging config option this htis was incorrect even before this change.","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d5ee8a8217d514c19d654a9acd52898073eb047c","unresolved":true,"context_lines":[{"line_number":318,"context_line":"            # We enforce the compute node where we create the instance to"},{"line_number":319,"context_line":"            # make sure we have one node on each compute."},{"line_number":320,"context_line":"            # This requires Nova API version 2.74 or higher."},{"line_number":321,"context_line":"            kwargs_server \u003d {\u0027host\u0027: node[\u0027host\u0027]}"},{"line_number":322,"context_line":"            # In case we want to run commands we will be injecting it via"},{"line_number":323,"context_line":"            # user_data which requires to setup the instance as validatable"},{"line_number":324,"context_line":"            # in tempest.common.compute.create_test_server"}],"source_content_type":"text/x-python","patch_set":2,"id":"cf6bfe0e_222e878b","line":321,"updated":"2025-06-19 00:17:00.000000000","message":"ok so th8is is where we are actully setting the expected host.","commit_id":"e918374c3e537a41976bbb65a3bdbbb0699366ba"}]}