)]}'
{"/PATCHSET_LEVEL":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"94876346f6c2b1c56076971083d6fd2656ce93b4","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"2af85698_43df10c1","updated":"2025-03-21 11:15:38.000000000","message":"by the way for bug fixes the pattern we should be takign is first submit a patch that repoduced the problem in a test, then submit a seconned commit on top that updates the test and fixes the probelm\n\n\nfor nova we have a regressions direcotry in our test that we add a new file to every time we are fixing a bug like this.\n\nwhile we dont strictly require it for every but we try to do this for the vast majority and i would like to see us do the same in watcher.\n\nhttps://github.com/openstack/nova/tree/master/nova/tests/functional/regressions","commit_id":"948b0101d0995e36a285614c444e3c592c5b2766"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"02309e9886fd474ccb4da408c19b4233fa4a125b","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":1,"id":"da4c7ded_b2743dc4","in_reply_to":"2af85698_43df10c1","updated":"2025-03-21 12:03:26.000000000","message":"sounds good, it\u0027ll try to add it, I pushed this to test is in a fresh devstack environment, but it\u0027s still work in progress, it does not seem to fix the issue as it is","commit_id":"948b0101d0995e36a285614c444e3c592c5b2766"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"965d0ebe43d386a406f3be5745d4f12d5e46ab48","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":5,"id":"db6676a8_caf6b2e7","updated":"2025-03-31 15:44:17.000000000","message":"so my general feedback is we should separate out a reproduce using the opportunistic test fixture to show tis broken and then address the issue in followup patches.","commit_id":"f41332de1b823797cb57c6085259b13326e9c3e2"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"3b79da324ae5013e2c151be2dad0ca660090a5ef","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"7621c406_bc22478c","updated":"2025-04-15 11:58:07.000000000","message":"I\u0027ve tested this together with https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/945325/12 (to make sure the stabilization strategy actually creates a real AP) in https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/947207 and these are my findings:\n\n- The db schema update is running fine:","commit_id":"cecf9a6a7d9fd514f6dc56e5139685227c3f3358"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"4bbaa325c47a1e06527281c697c08e8095140cdd","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":13,"id":"a32c3e64_5e65ced4","in_reply_to":"7621c406_bc22478c","updated":"2025-04-15 12:06:04.000000000","message":"I\u0027ve tested this together with https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/945325/12 (to make sure the stabilization strategy actually creates a real AP) in https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/947207 and these are my findings:\n\n- The db schema update is running fine (https://ef307f020103ee2d2a1b-594e2b182145c9c989109e5862503d2d.ssl.cf5.rackcdn.com/openstack/605d3a7c9d544fdfb60e9203566aabe1/controller/logs/devstacklog.txt):\n\n`2025-04-15 10:23:50.489 | INFO  [alembic.runtime.migration] Running upgrade 609bec748f2a -\u003e 15f7375ca737, change_efficiacy_indicator_decimals`\n\n- The strategy is created and deviation standard properly calculated (https://ef307f020103ee2d2a1b-594e2b182145c9c989109e5862503d2d.ssl.cf5.rackcdn.com/openstack/605d3a7c9d544fdfb60e9203566aabe1/controller/logs/screen-watcher-decision-engine.txt)\n\n`\nApr 15 10:47:40.692458 np0040462659 watcher-decision-engine[98573]: INFO watcher.decision_engine.strategy.strategies.workload_stabilization [None req-39768da0-51de-4e0f-8289-ab0999d409c1 None None] Standard deviation for instance_cpu_usage is 0.2503063541666666.\nApr 15 10:47:40.692458 np0040462659 watcher-decision-engine[98573]: INFO watcher.decision_engine.strategy.strategies.workload_stabilization [None req-39768da0-51de-4e0f-8289-ab0999d409c1 None None] Standard deviation of instance_cpu_usage exceeds appropriate threshold 0.1 by 0.2503063541666666.\n`\n\n- However, looking at tempest logs (https://ef307f020103ee2d2a1b-594e2b182145c9c989109e5862503d2d.ssl.cf5.rackcdn.com/openstack/605d3a7c9d544fdfb60e9203566aabe1/controller/logs/tempest_log.txt) the API seems to still be providing  0.0 as value:\n\n`\nResponse - Headers: {\u0027date\u0027: \u0027Tue, 15 Apr 2025 10:47:42 GMT\u0027, \u0027server\u0027: \u0027Apache/2.4.58 (Ubuntu)\u0027, \u0027vary\u0027: \u0027OpenStack-API-Version\u0027, \u0027openstack-api-minimum-version\u0027: \u00271.0\u0027, \u0027openstack-api-maximum-version\u0027: \u00271.4\u0027, \u0027openstack-api-version\u0027: \u0027infra-optim 1.4\u0027, \u0027content-length\u0027: \u00271195\u0027, \u0027content-type\u0027: \u0027application/json\u0027, \u0027connection\u0027: \u0027close\u0027, \u0027status\u0027: \u0027200\u0027, \u0027content-location\u0027: \u0027https://200.225.47.11/infra-optim/v1/action_plans?audit_uuid\u003da08c2891-d63b-49d3-b229-349ee583fb32\u0027}\n        Body: b\u0027{\"action_plans\": [{\"efficacy_indicators\": [{\"name\": \"instance_migrations_count\", \"description\": \"The number of VM migrations to be performed.\", \"unit\": null, \"value\": 1.0}, {\"name\": \"standard_deviation_before_audit\", \"description\": \"The value of original standard deviation.\", \"unit\": null, \"value\": 0.0}, {\"name\": \"standard_deviation_after_audit\", \"description\": \"The value of resulted standard deviation.\", \"unit\": null, \"value\": 0.0}, {\"name\": \"instances_count\", \"description\": \"The total number of audited instances in strategy.\", \"unit\": null, \"value\": 2.0}], \"uuid\": \"612bac6d-7e44-4d34-9bab-83d8d73607bb\", \"audit_uuid\": \"a08c2891-d63b-49d3-b229-349ee583fb32\", \"strategy_uuid\": \"73832c78-4554-42a4-99a9-0afc26af0827\", \"strategy_name\": \"workload_stabilization\", \"global_efficacy\": [{\"name\": \"live_migrations_count\", \"description\": \"Ratio of migrated virtual machines to audited virtual machines\", \"unit\": \"%\", \"value\": 50.0}], \"state\": \"RECOMMENDED\", \"links\": [{\"href\": \"https://200.225.47.11/v1/action_plans/612bac6d-7e44-4d34-9bab-83d8d73607bb\", \"rel\": \"self\"}, {\"href\": \"https://200.225.47.11/action_plans/612bac6d-7e44-4d34-9bab-83d8d73607bb\", \"rel\": \"bookmark\"}], \"updated_at\": null}]}\u0027\n`","commit_id":"cecf9a6a7d9fd514f6dc56e5139685227c3f3358"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"00a84a2526d2e7134f44d00172b9639d517222c4","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":13,"id":"07729ad5_7ae29972","in_reply_to":"a32c3e64_5e65ced4","updated":"2025-04-16 07:30:27.000000000","message":"thanks for looking Alfredo, there was indeed a bug which I fixed in PS 14, where in some cases it was still using the old column from the database. I\u0027ve run your DNM patch again and now the std is reported properly in the tempest logs (https://6e60353ab6408ff35f72-65a5b55aa83e6a6123b1ed34561cc44d.ssl.cf5.rackcdn.com/openstack/c56d1d2d38ac4de08be4fe724a1211fe/controller/logs/tempest_log.txt):\n```\n  Body: b\u0027{\"action_plans\": [{\"efficacy_indicators\": [{\"name\": \"instance_migrations_count\", \"description\": \"The number of VM migrations to be performed.\", \"unit\": null, \"value\": 1.0},\n {\"name\": \"standard_deviation_before_audit\", \"description\": \"The value of original standard deviation.\", \"unit\": null, \"value\": 0.2501},\n {\"name\": \"standard_deviation_after_audit\", \"description\": \"The value of resulted standard deviation.\", \"unit\": null, \"value\": 0.1001},\n {\"name\": \"instances_count\", \"description\": \"The total number of audited instances in strategy.\", \"unit\": null, \"value\": 2.0}]\n```","commit_id":"cecf9a6a7d9fd514f6dc56e5139685227c3f3358"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"6b2c7351d89010fff7623802f97cbaa7abb15e3f","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"cd7da131_0828edca","updated":"2025-04-23 09:49:15.000000000","message":"Just a doubt about the upgrade part of the alembic migration. Other than that, lgtm","commit_id":"a38d94c3a8efb715c09e1bc7e39513ffec9d086b"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"153901f6c3be5f71887db7b19529ed4e1edac9ee","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":16,"id":"d39a1fdd_29c666d5","updated":"2025-04-23 09:06:13.000000000","message":"Results from https://review.opendev.org/c/openstack/watcher-tempest-plugin/+/947207 with PS15:\n\nhttps://6e60353ab6408ff35f72-65a5b55aa83e6a6123b1ed34561cc44d.ssl.cf5.rackcdn.com/openstack/c56d1d2d38ac4de08be4fe724a1211fe/controller/logs/screen-watcher-decision-engine.txt\n\nStandard deviation for instance_cpu_usage is 0.25009999999999993.\nMigration of 661b8f0e-8c07-4809-a3b3-bd1a6ce854ee from ca2c7005-b9b4-4774-9c49-cbfe63c66d32 to d9af3515-4a38-45c4-bf21-84b4a7dc034d reduces standard deviation to 0.10010000000000002.\n\nAnd from tempest log https://6e60353ab6408ff35f72-65a5b55aa83e6a6123b1ed34561cc44d.ssl.cf5.rackcdn.com/openstack/c56d1d2d38ac4de08be4fe724a1211fe/controller/logs/tempest_log.txt\n\n        Body: b\u0027{\"action_plans\": [{\"efficacy_indicators\": [{\"name\": \"instance_migrations_count\", \"description\": \"The number of VM migrations to be performed.\", \"unit\": null, \"value\": 1.0}, {\"name\": \"standard_deviation_before_audit\", \"description\": \"The value of original standard deviation.\", \"unit\": null, \"value\": 0.2501}, {\"name\": \"standard_deviation_after_audit\", \"description\": \"The value of resulted standard deviation.\", \"unit\": null, \"value\": 0.1001}, {\"name\": \"instances_count\", \"description\": \"The total number of audited instances in strategy.\", \"unit\": null, \"value\": 2.0}], \"uuid\": \"a8c69f38-21fa-42d3-bdbf-ce9d2878cf40\", \"audit_uuid\": \"7a799a39-f8fa-4e5c-b965-c6856bde29a1\", \"strategy_uuid\": \"8eda5ce3-497c-43c9-919e-e525781b4610\", \"strategy_name\": \"workload_stabilization\", \"global_efficacy\": [{\"name\": \"live_migrations_count\", \"description\": \"Ratio of migrated virtual machines to audited virtual machines\", \"unit\": \"%\", \"value\": 50.0}], \"state\": \"RECOMMENDED\", \"links\": [{\"href\": \"https://10.0.19.230/v1/action_plans/a8c69f38-21fa-42d3-bdbf-ce9d2878cf40\", \"rel\": \"self\"}, {\"href\": \"https://10.0.19.230/action_plans/a8c69f38-21fa-42d3-bdbf-ce9d2878cf40\", \"rel\": \"bookmark\"}], \"updated_at\": null}]}\u0027 _log_request_full /opt/stack/tempest/tempest/lib/common/rest_client.py:484\n        \nNote:\n\n\n\"name\": \"standard_deviation_before_audit\", \"description\": \"The value of original standard deviation.\", \"unit\": null, \"value\": 0.2501\n\"name\": \"standard_deviation_after_audit\", \"description\": \"The value of resulted standard deviation.\", \"unit\": null, \"value\": 0.1001\n\nThis is the expected behavior.","commit_id":"a38d94c3a8efb715c09e1bc7e39513ffec9d086b"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"6f5e04f2dfec694983e705f43a10c3cd9b974308","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"9da7828a_16518b79","updated":"2025-05-09 11:16:48.000000000","message":"Thanks for the fix Joan. Nice work incluing references to the launchpad bug in the code and providing a good test coverage","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f1df1079db300d9005d3d3624b6527f5d9540af9","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"901b26f3_ad77dab2","updated":"2025-05-12 14:32:40.000000000","message":"i think we can proceed with this in its current form but we likely should have a follow up patch to do the migration form value to data if data is not set.","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"b2201b7a93455d66082d19bde43fd750951765c1","unresolved":false,"context_lines":[],"source_content_type":"","patch_set":23,"id":"4eede2f3_251f0cd3","updated":"2025-05-12 06:04:32.000000000","message":"lgtm","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"0499c0e5c3cf28fa39919520b2088e186ec83080","unresolved":true,"context_lines":[],"source_content_type":"","patch_set":23,"id":"c496b90b_5818f3be","updated":"2025-05-08 18:15:46.000000000","message":"nice work Joan, I have concern about the missing downgrade","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"}],"watcher/db/sqlalchemy/alembic/versions/15f7375ca737_change_efficiacy_indicator_decimals.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"965d0ebe43d386a406f3be5745d4f12d5e46ab48","unresolved":true,"context_lines":[{"line_number":18,"context_line":"                    type_\u003dsa.Numeric(precision\u003d10, scale\u003d2))"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"def downgrade():"},{"line_number":22,"context_line":"    op.alter_column(\u0027efficacy_indicators\u0027, \u0027value\u0027,"},{"line_number":23,"context_line":"                    type_\u003dsa.Numeric(precision\u003d10, scale\u003d0))"}],"source_content_type":"text/x-python","patch_set":5,"id":"e6e8a65a_07f1d3b1","line":21,"updated":"2025-03-31 15:44:17.000000000","message":"we do not support downgrade so you should not add a downgrade hook.","commit_id":"f41332de1b823797cb57c6085259b13326e9c3e2"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"49e58ddbf4d845c0c3b9c0807095196b4031d001","unresolved":false,"context_lines":[{"line_number":18,"context_line":"                    type_\u003dsa.Numeric(precision\u003d10, scale\u003d2))"},{"line_number":19,"context_line":""},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"def downgrade():"},{"line_number":22,"context_line":"    op.alter_column(\u0027efficacy_indicators\u0027, \u0027value\u0027,"},{"line_number":23,"context_line":"                    type_\u003dsa.Numeric(precision\u003d10, scale\u003d0))"}],"source_content_type":"text/x-python","patch_set":5,"id":"2b91ad75_e628ec37","line":21,"in_reply_to":"e6e8a65a_07f1d3b1","updated":"2025-04-14 14:35:39.000000000","message":"ack, removed it","commit_id":"f41332de1b823797cb57c6085259b13326e9c3e2"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"6b2c7351d89010fff7623802f97cbaa7abb15e3f","unresolved":true,"context_lines":[{"line_number":19,"context_line":"                 )"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    # create table view"},{"line_number":22,"context_line":"    t_efficacy_indicator \u003d sa.Table("},{"line_number":23,"context_line":"            \u0027efficacy_indicators\u0027,"},{"line_number":24,"context_line":"            sa.MetaData(),"},{"line_number":25,"context_line":"            sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":16,"id":"9e7bf2a6_6d5cc845","line":22,"updated":"2025-04-23 09:49:15.000000000","message":"I wonder if there is a better way to update the table to avoid creating this view.\n\nMaybe a similar approach to https://github.com/openstack/watcher/blob/master/watcher/db/sqlalchemy/alembic/versions/a86240e89a29_.py#L19-L31 ?","commit_id":"a38d94c3a8efb715c09e1bc7e39513ffec9d086b"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"d3c0832eb82b6a95d92fff2ebc9c4509a2c79e36","unresolved":false,"context_lines":[{"line_number":19,"context_line":"                 )"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    # create table view"},{"line_number":22,"context_line":"    t_efficacy_indicator \u003d sa.Table("},{"line_number":23,"context_line":"            \u0027efficacy_indicators\u0027,"},{"line_number":24,"context_line":"            sa.MetaData(),"},{"line_number":25,"context_line":"            sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":16,"id":"093c128c_210cbbcd","line":22,"in_reply_to":"03aa1a62_57265c0b","updated":"2025-04-23 14:44:20.000000000","message":"Sorry, i missed your links to nova, got it.","commit_id":"a38d94c3a8efb715c09e1bc7e39513ffec9d086b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"0fe5608cf86fd9a290987e041f37162f1b2c6d5d","unresolved":false,"context_lines":[{"line_number":19,"context_line":"                 )"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    # create table view"},{"line_number":22,"context_line":"    t_efficacy_indicator \u003d sa.Table("},{"line_number":23,"context_line":"            \u0027efficacy_indicators\u0027,"},{"line_number":24,"context_line":"            sa.MetaData(),"},{"line_number":25,"context_line":"            sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":16,"id":"1c4c3f4a_2ef6e8ad","line":22,"in_reply_to":"093c128c_210cbbcd","updated":"2025-04-23 20:45:05.000000000","message":"ya so most project have a db sync command that does the schmea migration\n\nand then a seperate comamdn for data migrations.\n\nyou can see the same in cinder and ironic \n\nhttps://github.com/openstack/cinder/blob/master/cinder/cmd/manage.py#L158-L162\n\nhttps://github.com/openstack/ironic/blob/3024e6470fc8d7f313f966ae4036ec27e6523d6b/ironic/command/dbsync.py#L136\n\nfor distributed service that can have a lot of data its sometimes preferred vs inline data migratons \n\nhttps://codesearch.opendev.org/?q\u003ddef%20online_data_migrations\u0026i\u003dnope\u0026literal\u003dnope\u0026files\u003d\u0026excludeFiles\u003d\u0026repos\u003d\n\nin most project the only do inline migration or requrie offline data migration which is even less operator-friendly.","commit_id":"a38d94c3a8efb715c09e1bc7e39513ffec9d086b"},{"author":{"_account_id":16312,"name":"Alfredo Moralejo","email":"amoralej@redhat.com","username":"amoralej"},"change_message_id":"186f70da2f68c9efe8dd47192a2afba4f1f1e7d2","unresolved":false,"context_lines":[{"line_number":19,"context_line":"                 )"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    # create table view"},{"line_number":22,"context_line":"    t_efficacy_indicator \u003d sa.Table("},{"line_number":23,"context_line":"            \u0027efficacy_indicators\u0027,"},{"line_number":24,"context_line":"            sa.MetaData(),"},{"line_number":25,"context_line":"            sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":16,"id":"03aa1a62_57265c0b","line":22,"in_reply_to":"4ed0825c_6912c5ce","updated":"2025-04-23 14:41:57.000000000","message":"\u003e oneline data migration can be provided however they must be done as a seperate command/action then cannot be in the alembic migration script like this\n\nThat means in a different alembic migration file or out of alembic totally? Does openstack provides another mechanism to run migrations during updates?\n\nJust asking, for this particular case (given that it\u0027s easy in terms of data types), isn\u0027t cleaner to copy data from old to new column  instead of providing in-code way to handle both columns depending on the content for each audit?","commit_id":"a38d94c3a8efb715c09e1bc7e39513ffec9d086b"},{"author":{"_account_id":12393,"name":"chandan kumar","display_name":"Chandan Kumar","email":"chkumar@redhat.com","username":"chkumar246"},"change_message_id":"57c1f3b65ba331ea0bcaad48c681a15c101655f4","unresolved":true,"context_lines":[{"line_number":19,"context_line":"                 )"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    # create table view"},{"line_number":22,"context_line":"    t_efficacy_indicator \u003d sa.Table("},{"line_number":23,"context_line":"            \u0027efficacy_indicators\u0027,"},{"line_number":24,"context_line":"            sa.MetaData(),"},{"line_number":25,"context_line":"            sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":16,"id":"e55f8037_facf9dd5","line":22,"in_reply_to":"9e7bf2a6_6d5cc845","updated":"2025-04-23 10:23:40.000000000","message":"i had the same question. I assume this file is generated by this https://alembic.sqlalchemy.org/en/latest/tutorial.html#create-a-migration-script","commit_id":"a38d94c3a8efb715c09e1bc7e39513ffec9d086b"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"a6875b7f7eb9b8f17fc60df4710f08c8e29d399f","unresolved":true,"context_lines":[{"line_number":19,"context_line":"                 )"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    # create table view"},{"line_number":22,"context_line":"    t_efficacy_indicator \u003d sa.Table("},{"line_number":23,"context_line":"            \u0027efficacy_indicators\u0027,"},{"line_number":24,"context_line":"            sa.MetaData(),"},{"line_number":25,"context_line":"            sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":16,"id":"d8653f7a_fa0f1452","line":22,"in_reply_to":"9e7bf2a6_6d5cc845","updated":"2025-04-23 10:24:10.000000000","message":"removed the table view in PS 17","commit_id":"a38d94c3a8efb715c09e1bc7e39513ffec9d086b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fd1c4bbdb0e527f390b9a3edc5be931584607545","unresolved":true,"context_lines":[{"line_number":19,"context_line":"                 )"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    # create table view"},{"line_number":22,"context_line":"    t_efficacy_indicator \u003d sa.Table("},{"line_number":23,"context_line":"            \u0027efficacy_indicators\u0027,"},{"line_number":24,"context_line":"            sa.MetaData(),"},{"line_number":25,"context_line":"            sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":16,"id":"df96226e_3284e811","line":22,"in_reply_to":"d8653f7a_fa0f1452","updated":"2025-04-23 11:24:48.000000000","message":"to be clear we shoudl not be doing a data rewite as part of the schema migration.\n\nthe schema mication shoudl just add the new column\n\nthe code shoudl be updated ot alter how we load the data form the tabel to prefer the new colum over the old and only write to the new collum.\n\nwe can and should save back the data on load if the new colume is not populated so that we dont need the fallback on subsequent loads.\n\n\noneline data migration can be provided however they must be done as a seperate command/action then cannot be in the alembic migration script like this","commit_id":"a38d94c3a8efb715c09e1bc7e39513ffec9d086b"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"d7323d0eb7c16d5aca28b6c9cb42e34c60724640","unresolved":false,"context_lines":[{"line_number":19,"context_line":"                 )"},{"line_number":20,"context_line":""},{"line_number":21,"context_line":"    # create table view"},{"line_number":22,"context_line":"    t_efficacy_indicator \u003d sa.Table("},{"line_number":23,"context_line":"            \u0027efficacy_indicators\u0027,"},{"line_number":24,"context_line":"            sa.MetaData(),"},{"line_number":25,"context_line":"            sa.Column(\u0027id\u0027, sa.Integer, primary_key\u003dTrue,"}],"source_content_type":"text/x-python","patch_set":16,"id":"4ed0825c_6912c5ce","line":22,"in_reply_to":"df96226e_3284e811","updated":"2025-04-23 12:32:13.000000000","message":"Acknowledged","commit_id":"a38d94c3a8efb715c09e1bc7e39513ffec9d086b"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"fd1c4bbdb0e527f390b9a3edc5be931584607545","unresolved":true,"context_lines":[{"line_number":20,"context_line":"                  sa.Column(\u0027data\u0027, sa.Float())"},{"line_number":21,"context_line":"                 )"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    # get the existing data in the \u0027value\u0027 column"},{"line_number":24,"context_line":"    conn \u003d op.get_bind()"},{"line_number":25,"context_line":"    session \u003d  sessionmaker()"},{"line_number":26,"context_line":"    s \u003d session(bind\u003dconn)"},{"line_number":27,"context_line":"    stmt \u003d sa.select(models.EfficacyIndicator.id,"},{"line_number":28,"context_line":"                     models.EfficacyIndicator.value)"},{"line_number":29,"context_line":"    indicators \u003d s.execute(stmt).all()"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    new_data \u003d [{\"id\": id_, \"data\": data} for id_, data in indicators]"},{"line_number":32,"context_line":"    s.execute(sa.update(models.EfficacyIndicator), new_data)"}],"source_content_type":"text/x-python","patch_set":17,"id":"99d18d95_fa80932b","line":32,"range":{"start_line":23,"start_character":0,"end_line":32,"end_character":60},"updated":"2025-04-23 11:24:48.000000000","message":"this is all still incorrect to do in a schema migraiton.\n\nno data can be migrated when doing this schema migration.\n\nthe two have to be done seperatly.\n\nschema migration are ment to be apply able whil runign the old version fo the software that know nothing about the new table layout.\n\nthis is related to the grenade thory of upgrade\n\nhttps://opendev.org/openstack/grenade/src/branch/master#Theory%20of%20Upgrade\n\n\nwatcher has never implemted an online data migration before so we have to look to nova and opther project for examples\n\nhttps://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L198-L224\nhttps://github.com/openstack/nova/blob/master/nova/cmd/manage.py#L597-L655\n\nwhile we coudl intoduce this concept to watcher i think it would be better ot judt do the lazy data migration on load given its unlikely that anyone will ever look at the old audits/actionplans ot need to migrate the data in the first place.\n\nthey are likely to get perged out over time as tehy are archived and we can avoid the data migration entirly for those records.","commit_id":"0fa7cb00a9a08f129441430171eeb2499e20e621"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d5bbd152a02077445be865c11a9edb2a2af16b99","unresolved":false,"context_lines":[{"line_number":20,"context_line":"                  sa.Column(\u0027data\u0027, sa.Float())"},{"line_number":21,"context_line":"                 )"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    # get the existing data in the \u0027value\u0027 column"},{"line_number":24,"context_line":"    conn \u003d op.get_bind()"},{"line_number":25,"context_line":"    session \u003d  sessionmaker()"},{"line_number":26,"context_line":"    s \u003d session(bind\u003dconn)"},{"line_number":27,"context_line":"    stmt \u003d sa.select(models.EfficacyIndicator.id,"},{"line_number":28,"context_line":"                     models.EfficacyIndicator.value)"},{"line_number":29,"context_line":"    indicators \u003d s.execute(stmt).all()"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    new_data \u003d [{\"id\": id_, \"data\": data} for id_, data in indicators]"},{"line_number":32,"context_line":"    s.execute(sa.update(models.EfficacyIndicator), new_data)"}],"source_content_type":"text/x-python","patch_set":17,"id":"d2f18136_c1e72f97","line":32,"range":{"start_line":23,"start_character":0,"end_line":32,"end_character":60},"in_reply_to":"813bf68f_704609e1","updated":"2025-05-08 19:03:11.000000000","message":"Acknowledged","commit_id":"0fa7cb00a9a08f129441430171eeb2499e20e621"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"93acafba56e945c48511dc63f08601a4236f7ef9","unresolved":true,"context_lines":[{"line_number":20,"context_line":"                  sa.Column(\u0027data\u0027, sa.Float())"},{"line_number":21,"context_line":"                 )"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    # get the existing data in the \u0027value\u0027 column"},{"line_number":24,"context_line":"    conn \u003d op.get_bind()"},{"line_number":25,"context_line":"    session \u003d  sessionmaker()"},{"line_number":26,"context_line":"    s \u003d session(bind\u003dconn)"},{"line_number":27,"context_line":"    stmt \u003d sa.select(models.EfficacyIndicator.id,"},{"line_number":28,"context_line":"                     models.EfficacyIndicator.value)"},{"line_number":29,"context_line":"    indicators \u003d s.execute(stmt).all()"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    new_data \u003d [{\"id\": id_, \"data\": data} for id_, data in indicators]"},{"line_number":32,"context_line":"    s.execute(sa.update(models.EfficacyIndicator), new_data)"}],"source_content_type":"text/x-python","patch_set":17,"id":"ba515d25_451cfa31","line":32,"range":{"start_line":23,"start_character":0,"end_line":32,"end_character":60},"in_reply_to":"99d18d95_fa80932b","updated":"2025-04-23 11:27:57.000000000","message":"i ment to link tonova docs on db migration \n\nhttps://docs.openstack.org/nova/latest/reference/database-migrations.html\n\n\"Inline data migrations\" are generaly preferd over \"Online data migrations\"\nbecasue the spread the load over time but there are some cases where we really do need to do it all at once.\n\nthat genrlly when we are removing the fallback to the old way and we provide a \n\"blocking\" Online data migration in that case to signal that you cannot upgrade until you have completed this migration.","commit_id":"0fa7cb00a9a08f129441430171eeb2499e20e621"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"d7323d0eb7c16d5aca28b6c9cb42e34c60724640","unresolved":true,"context_lines":[{"line_number":20,"context_line":"                  sa.Column(\u0027data\u0027, sa.Float())"},{"line_number":21,"context_line":"                 )"},{"line_number":22,"context_line":""},{"line_number":23,"context_line":"    # get the existing data in the \u0027value\u0027 column"},{"line_number":24,"context_line":"    conn \u003d op.get_bind()"},{"line_number":25,"context_line":"    session \u003d  sessionmaker()"},{"line_number":26,"context_line":"    s \u003d session(bind\u003dconn)"},{"line_number":27,"context_line":"    stmt \u003d sa.select(models.EfficacyIndicator.id,"},{"line_number":28,"context_line":"                     models.EfficacyIndicator.value)"},{"line_number":29,"context_line":"    indicators \u003d s.execute(stmt).all()"},{"line_number":30,"context_line":""},{"line_number":31,"context_line":"    new_data \u003d [{\"id\": id_, \"data\": data} for id_, data in indicators]"},{"line_number":32,"context_line":"    s.execute(sa.update(models.EfficacyIndicator), new_data)"}],"source_content_type":"text/x-python","patch_set":17,"id":"813bf68f_704609e1","line":32,"range":{"start_line":23,"start_character":0,"end_line":32,"end_character":60},"in_reply_to":"ba515d25_451cfa31","updated":"2025-04-23 12:32:13.000000000","message":"thanks for the links, considering the nature of the bug this patch is trying to solve, doing only an inline migration should be ok, I\u0027ll remove the mentioned lines and only add the new column in the migration script, I\u0027ll also make sure it using the \"value\" column as fallback when reading from the db","commit_id":"0fa7cb00a9a08f129441430171eeb2499e20e621"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d5bbd152a02077445be865c11a9edb2a2af16b99","unresolved":true,"context_lines":[{"line_number":16,"context_line":"def upgrade():"},{"line_number":17,"context_line":"    op.add_column(\u0027efficacy_indicators\u0027,"},{"line_number":18,"context_line":"                  sa.Column(\u0027data\u0027, sa.Float())"},{"line_number":19,"context_line":"                 )"},{"line_number":20,"context_line":""}],"source_content_type":"text/x-python","patch_set":22,"id":"1194eb3d_e92cccff","line":19,"updated":"2025-05-08 19:03:11.000000000","message":"+1","commit_id":"893eb3369b0b43a7cfead6ecbf95baa5332fc856"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"0499c0e5c3cf28fa39919520b2088e186ec83080","unresolved":true,"context_lines":[{"line_number":13,"context_line":"from alembic import op"},{"line_number":14,"context_line":"import sqlalchemy as sa"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"def upgrade():"},{"line_number":17,"context_line":"    op.add_column(\u0027efficacy_indicators\u0027,"},{"line_number":18,"context_line":"                  sa.Column(\u0027data\u0027, sa.Float())"},{"line_number":19,"context_line":"                 )"}],"source_content_type":"text/x-python","patch_set":23,"id":"20911536_08d9e7f7","line":16,"updated":"2025-05-08 18:15:46.000000000","message":"I think that we are missing the downgrade() here.","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d5bbd152a02077445be865c11a9edb2a2af16b99","unresolved":true,"context_lines":[{"line_number":13,"context_line":"from alembic import op"},{"line_number":14,"context_line":"import sqlalchemy as sa"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"def upgrade():"},{"line_number":17,"context_line":"    op.add_column(\u0027efficacy_indicators\u0027,"},{"line_number":18,"context_line":"                  sa.Column(\u0027data\u0027, sa.Float())"},{"line_number":19,"context_line":"                 )"}],"source_content_type":"text/x-python","patch_set":23,"id":"9f0643d4_cbd042a3","line":16,"in_reply_to":"20911536_08d9e7f7","updated":"2025-05-08 19:03:11.000000000","message":"nope openstack does nto supprot downgrade any more.\n\nnova and some other project never supported them but\n\ndowngrade were intended to be remvoed across openstack 10 years ago.\nhttps://review.opendev.org/c/openstack/openstack-specs/+/152337\n\nthis has come up several tims since but as a comuntiy our over all stance is downgrade are not tested or formaly supproted and instead you should restore form a backup.","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"6f5e04f2dfec694983e705f43a10c3cd9b974308","unresolved":true,"context_lines":[{"line_number":13,"context_line":"from alembic import op"},{"line_number":14,"context_line":"import sqlalchemy as sa"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"def upgrade():"},{"line_number":17,"context_line":"    op.add_column(\u0027efficacy_indicators\u0027,"},{"line_number":18,"context_line":"                  sa.Column(\u0027data\u0027, sa.Float())"},{"line_number":19,"context_line":"                 )"}],"source_content_type":"text/x-python","patch_set":23,"id":"54fbe3f8_65afb77f","line":16,"in_reply_to":"9f0643d4_cbd042a3","updated":"2025-05-09 11:16:48.000000000","message":"interesting that some project(s) still keep including the downgrade option. i propose some of them in the past years XD\nbut if is not required at all, that\u0027s fine","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"fb6c10c3c6543ba551f0b056a00d8e7dc85d023d","unresolved":true,"context_lines":[{"line_number":13,"context_line":"from alembic import op"},{"line_number":14,"context_line":"import sqlalchemy as sa"},{"line_number":15,"context_line":""},{"line_number":16,"context_line":"def upgrade():"},{"line_number":17,"context_line":"    op.add_column(\u0027efficacy_indicators\u0027,"},{"line_number":18,"context_line":"                  sa.Column(\u0027data\u0027, sa.Float())"},{"line_number":19,"context_line":"                 )"}],"source_content_type":"text/x-python","patch_set":23,"id":"59ec4912_7840f22d","line":16,"in_reply_to":"9f0643d4_cbd042a3","updated":"2025-05-09 06:21:24.000000000","message":"thanks for the link Sean!","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"}],"watcher/db/sqlalchemy/api.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"f1df1079db300d9005d3d3624b6527f5d9540af9","unresolved":true,"context_lines":[{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    # ### EFFICACY INDICATORS ### #"},{"line_number":906,"context_line":""},{"line_number":907,"context_line":"    def get_efficacy_indicator_list(self, *args, **kwargs):"},{"line_number":908,"context_line":"        eff_ind_models \u003d self._get_model_list("},{"line_number":909,"context_line":"            models.EfficacyIndicator,"},{"line_number":910,"context_line":"            self._add_efficacy_indicators_filters,"}],"source_content_type":"text/x-python","patch_set":23,"id":"f2d78e9c_0728d6b3","line":907,"updated":"2025-05-12 14:32:40.000000000","message":"This is not entirly correct.\n\nit will work for what we need however you are not doing the data migration on load.\n\n\nso the normal patern is to check the new colume if, its not empty use it\nif it is load form the old one and save back to the new column.\n\nthis those the data migration over time and mean we eveutlly can drop  the old column.","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"e25c2919f6bc0d8161e6961afc7978c40c94de81","unresolved":true,"context_lines":[{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    # ### EFFICACY INDICATORS ### #"},{"line_number":906,"context_line":""},{"line_number":907,"context_line":"    def get_efficacy_indicator_list(self, *args, **kwargs):"},{"line_number":908,"context_line":"        eff_ind_models \u003d self._get_model_list("},{"line_number":909,"context_line":"            models.EfficacyIndicator,"},{"line_number":910,"context_line":"            self._add_efficacy_indicators_filters,"}],"source_content_type":"text/x-python","patch_set":23,"id":"7799db33_121bd333","line":907,"in_reply_to":"21558e9d_ffc51070","updated":"2025-05-13 10:52:32.000000000","message":"ok I think I got it now, when loading using the old column,we should update the object in the database to store the value in the new column, is that right?","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"3d8a7f0a00d90b690db350003459773cee1351d5","unresolved":false,"context_lines":[{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    # ### EFFICACY INDICATORS ### #"},{"line_number":906,"context_line":""},{"line_number":907,"context_line":"    def get_efficacy_indicator_list(self, *args, **kwargs):"},{"line_number":908,"context_line":"        eff_ind_models \u003d self._get_model_list("},{"line_number":909,"context_line":"            models.EfficacyIndicator,"},{"line_number":910,"context_line":"            self._add_efficacy_indicators_filters,"}],"source_content_type":"text/x-python","patch_set":23,"id":"47e7f2bb_321c1209","line":907,"in_reply_to":"49345d65_fb5bc679","updated":"2025-05-13 11:00:30.000000000","message":"thanks, now that I understand I\u0027ll create a follow-up patch with the change","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"550aed1bbbfbfa10e0188f0a94e1095cc2120e05","unresolved":true,"context_lines":[{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    # ### EFFICACY INDICATORS ### #"},{"line_number":906,"context_line":""},{"line_number":907,"context_line":"    def get_efficacy_indicator_list(self, *args, **kwargs):"},{"line_number":908,"context_line":"        eff_ind_models \u003d self._get_model_list("},{"line_number":909,"context_line":"            models.EfficacyIndicator,"},{"line_number":910,"context_line":"            self._add_efficacy_indicators_filters,"}],"source_content_type":"text/x-python","patch_set":23,"id":"f3c46f80_656047ee","line":907,"in_reply_to":"533658e7_ca8b53d4","updated":"2025-05-13 10:44:35.000000000","message":"yes if you have to load it form the old location you are ment to write it to the new localtion so that we can eventurlly drop this old colunmn and remove the fallback code.\n\nin other words you shoudl be doing the data migration on load if required.\n\nthe merged code is fine and will work but it does nto do the migration over time whihch means that the only way to eveunctly drop the table is to archive the past results and wait for all usage of the old colunm to age out.","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"3f068b4b646aaec7e9825136c983490457a073a2","unresolved":true,"context_lines":[{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    # ### EFFICACY INDICATORS ### #"},{"line_number":906,"context_line":""},{"line_number":907,"context_line":"    def get_efficacy_indicator_list(self, *args, **kwargs):"},{"line_number":908,"context_line":"        eff_ind_models \u003d self._get_model_list("},{"line_number":909,"context_line":"            models.EfficacyIndicator,"},{"line_number":910,"context_line":"            self._add_efficacy_indicators_filters,"}],"source_content_type":"text/x-python","patch_set":23,"id":"49345d65_fb5bc679","line":907,"in_reply_to":"7799db33_121bd333","updated":"2025-05-13 10:58:12.000000000","message":"yes ideally. as i said its funcitonal as it is without htat but we would need to provide either an online migration that would have to be run before we drop the old column or we can spread it out over time by migrating object on load.\n\neither works. \n\ni didnt think this was worth holdign the patch on but we normally do one or the other in the patch when we add a new column like this.\n\nit only becomes imporant when we are addign teh contract migration to drop the old db column.","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"caa4ae88db6846a64867f4f4daa0a0e338452e6c","unresolved":true,"context_lines":[{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    # ### EFFICACY INDICATORS ### #"},{"line_number":906,"context_line":""},{"line_number":907,"context_line":"    def get_efficacy_indicator_list(self, *args, **kwargs):"},{"line_number":908,"context_line":"        eff_ind_models \u003d self._get_model_list("},{"line_number":909,"context_line":"            models.EfficacyIndicator,"},{"line_number":910,"context_line":"            self._add_efficacy_indicators_filters,"}],"source_content_type":"text/x-python","patch_set":23,"id":"533658e7_ca8b53d4","line":907,"in_reply_to":"f2d78e9c_0728d6b3","updated":"2025-05-13 07:05:14.000000000","message":"sorry, but I don\u0027t see what is missing here. in this patch whenever an efficacy_indicator is loaded from the db it check if the `data` column is not empty and loads it, it it\u0027s empty, it loads the `value` column but it stores it in the `data` one, is there another case that I\u0027m missing?","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"4fd349b79242a287aa01822e18fa4f9f85082905","unresolved":true,"context_lines":[{"line_number":904,"context_line":""},{"line_number":905,"context_line":"    # ### EFFICACY INDICATORS ### #"},{"line_number":906,"context_line":""},{"line_number":907,"context_line":"    def get_efficacy_indicator_list(self, *args, **kwargs):"},{"line_number":908,"context_line":"        eff_ind_models \u003d self._get_model_list("},{"line_number":909,"context_line":"            models.EfficacyIndicator,"},{"line_number":910,"context_line":"            self._add_efficacy_indicators_filters,"}],"source_content_type":"text/x-python","patch_set":23,"id":"21558e9d_ffc51070","line":907,"in_reply_to":"f3c46f80_656047ee","updated":"2025-05-13 10:45:51.000000000","message":"the way it currently funcitons effectivly assume that we will provide a seperate online data migration script which is also an option.","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"}],"watcher/db/sqlalchemy/models.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"d9146a5dd91bb0b9b6840fe96b157dd9a4fb7810","unresolved":true,"context_lines":[{"line_number":238,"context_line":"    name \u003d Column(String(63))"},{"line_number":239,"context_line":"    description \u003d Column(String(255), nullable\u003dTrue)"},{"line_number":240,"context_line":"    unit \u003d Column(String(63), nullable\u003dTrue)"},{"line_number":241,"context_line":"    value \u003d Column(Numeric(scale\u003d3))"},{"line_number":242,"context_line":"    action_plan_id \u003d Column(Integer, ForeignKey(\u0027action_plans.id\u0027),"},{"line_number":243,"context_line":"                            nullable\u003dFalse)"},{"line_number":244,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"6f5cf729_d7a71d11","line":241,"updated":"2025-03-21 11:13:14.000000000","message":"is this a db schema change or just a bodel change\n\nchanging String(63) to String(62) or 255 above would be a schmea change and require a db migration\n\nwe arae generally not allowed to update any colume in place and to supprot upgrade we need to add a new colume deprecate teh old and then remove it 2-4 cycles later after we are sure that no supported versions are still using it.\n\nso have you confrimed this does not change the schema or the data layout and is only a change in the model.\n\nwe will need to test this with data already in the db.\n\nwe shoudl have some level of testing for this too.","commit_id":"948b0101d0995e36a285614c444e3c592c5b2766"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"49e58ddbf4d845c0c3b9c0807095196b4031d001","unresolved":true,"context_lines":[{"line_number":238,"context_line":"    name \u003d Column(String(63))"},{"line_number":239,"context_line":"    description \u003d Column(String(255), nullable\u003dTrue)"},{"line_number":240,"context_line":"    unit \u003d Column(String(63), nullable\u003dTrue)"},{"line_number":241,"context_line":"    value \u003d Column(Numeric(scale\u003d3))"},{"line_number":242,"context_line":"    action_plan_id \u003d Column(Integer, ForeignKey(\u0027action_plans.id\u0027),"},{"line_number":243,"context_line":"                            nullable\u003dFalse)"},{"line_number":244,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"c0ee1c6f_650dbac0","line":241,"in_reply_to":"1de37ff2_704e0c51","updated":"2025-04-14 14:35:39.000000000","message":"in PS 8 I changed the model to include a new column and deprecate the old one as you mentioned, also added some level of testing for db migration","commit_id":"948b0101d0995e36a285614c444e3c592c5b2766"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ab519e5fc4b5059d3f1b10bad54959a27a9ada0d","unresolved":true,"context_lines":[{"line_number":238,"context_line":"    name \u003d Column(String(63))"},{"line_number":239,"context_line":"    description \u003d Column(String(255), nullable\u003dTrue)"},{"line_number":240,"context_line":"    unit \u003d Column(String(63), nullable\u003dTrue)"},{"line_number":241,"context_line":"    value \u003d Column(Numeric(scale\u003d3))"},{"line_number":242,"context_line":"    action_plan_id \u003d Column(Integer, ForeignKey(\u0027action_plans.id\u0027),"},{"line_number":243,"context_line":"                            nullable\u003dFalse)"},{"line_number":244,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"1de37ff2_704e0c51","line":241,"in_reply_to":"6f5cf729_d7a71d11","updated":"2025-03-21 13:58:39.000000000","message":"I need to look into how to handle upgrades, because the cause of the bug is the chema itself. I\u0027ll need to iterate on this as per my local testing it\u0027s not enough to fix the bug, the db created including this patch still has no decimal places, so I\u0027m still missing something here (testing is obviously still pending as well)","commit_id":"948b0101d0995e36a285614c444e3c592c5b2766"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2cae81bc3e8239db68d15951c1b4f5b72ca13eca","unresolved":false,"context_lines":[{"line_number":238,"context_line":"    name \u003d Column(String(63))"},{"line_number":239,"context_line":"    description \u003d Column(String(255), nullable\u003dTrue)"},{"line_number":240,"context_line":"    unit \u003d Column(String(63), nullable\u003dTrue)"},{"line_number":241,"context_line":"    value \u003d Column(Numeric(scale\u003d3))"},{"line_number":242,"context_line":"    action_plan_id \u003d Column(Integer, ForeignKey(\u0027action_plans.id\u0027),"},{"line_number":243,"context_line":"                            nullable\u003dFalse)"},{"line_number":244,"context_line":""}],"source_content_type":"text/x-python","patch_set":1,"id":"f192476e_a279f79f","line":241,"in_reply_to":"c0ee1c6f_650dbac0","updated":"2025-04-15 12:55:31.000000000","message":"Acknowledged","commit_id":"948b0101d0995e36a285614c444e3c592c5b2766"},{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"2cae81bc3e8239db68d15951c1b4f5b72ca13eca","unresolved":true,"context_lines":[{"line_number":241,"context_line":"    # this column is deprecated due to bug"},{"line_number":242,"context_line":"    # https://bugs.launchpad.net/watcher/+bug/2103458"},{"line_number":243,"context_line":"    value \u003d Column(Numeric())"},{"line_number":244,"context_line":"    value_decimal \u003d Column(Float())"},{"line_number":245,"context_line":"    action_plan_id \u003d Column(Integer, ForeignKey(\u0027action_plans.id\u0027),"},{"line_number":246,"context_line":"                            nullable\u003dFalse)"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"7e20ba39_1e6c2ace","line":244,"range":{"start_line":244,"start_character":4,"end_line":244,"end_character":35},"updated":"2025-04-15 12:55:31.000000000","message":"since its weid to go form a value field that is treated as a decimal\n\nto a value_decimal filed that is used as a float\n\ni would suggest using `data` or `kpi` instead of `value_decimal` just to aovid the ambiguity  over the type vs the name","commit_id":"cecf9a6a7d9fd514f6dc56e5139685227c3f3358"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"a6875b7f7eb9b8f17fc60df4710f08c8e29d399f","unresolved":false,"context_lines":[{"line_number":241,"context_line":"    # this column is deprecated due to bug"},{"line_number":242,"context_line":"    # https://bugs.launchpad.net/watcher/+bug/2103458"},{"line_number":243,"context_line":"    value \u003d Column(Numeric())"},{"line_number":244,"context_line":"    value_decimal \u003d Column(Float())"},{"line_number":245,"context_line":"    action_plan_id \u003d Column(Integer, ForeignKey(\u0027action_plans.id\u0027),"},{"line_number":246,"context_line":"                            nullable\u003dFalse)"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"c82748cd_cea39542","line":244,"range":{"start_line":244,"start_character":4,"end_line":244,"end_character":35},"in_reply_to":"39070285_869cb069","updated":"2025-04-23 10:24:10.000000000","message":"Done","commit_id":"cecf9a6a7d9fd514f6dc56e5139685227c3f3358"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ce8c15eb445aa01ef6c28f0808c764f0bcf36449","unresolved":true,"context_lines":[{"line_number":241,"context_line":"    # this column is deprecated due to bug"},{"line_number":242,"context_line":"    # https://bugs.launchpad.net/watcher/+bug/2103458"},{"line_number":243,"context_line":"    value \u003d Column(Numeric())"},{"line_number":244,"context_line":"    value_decimal \u003d Column(Float())"},{"line_number":245,"context_line":"    action_plan_id \u003d Column(Integer, ForeignKey(\u0027action_plans.id\u0027),"},{"line_number":246,"context_line":"                            nullable\u003dFalse)"},{"line_number":247,"context_line":""}],"source_content_type":"text/x-python","patch_set":13,"id":"39070285_869cb069","line":244,"range":{"start_line":244,"start_character":4,"end_line":244,"end_character":35},"in_reply_to":"7e20ba39_1e6c2ace","updated":"2025-04-15 15:10:31.000000000","message":"yes, the name I used is not great, I changed it to use `data`","commit_id":"cecf9a6a7d9fd514f6dc56e5139685227c3f3358"}],"watcher/tests/db/test_efficacy_indicator.py":[{"author":{"_account_id":11604,"name":"sean mooney","email":"smooney@redhat.com","username":"sean-k-mooney"},"change_message_id":"965d0ebe43d386a406f3be5745d4f12d5e46ab48","unresolved":true,"context_lines":[{"line_number":410,"context_line":"                          utils.create_test_efficacy_indicator,"},{"line_number":411,"context_line":"                          id\u003d2, uuid\u003duuid)"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"class MySQLDbEfficacyIndicatorTestCase(base.MySQLDbTestCase):"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"    FAKE_OLDER_DATE \u003d \u00272014-01-01T09:52:05.219414\u0027"},{"line_number":416,"context_line":"    FAKE_OLD_DATE \u003d \u00272015-01-01T09:52:05.219414\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"3b47654d_45dab1ed","line":413,"updated":"2025-03-31 15:44:17.000000000","message":"testing this properly is actully moer complet then this.\n\nwe need to test the migration was wel ideally with real data.\n\nso we need to have a test that will create the  db with the db schema piror to this change and insert data.\n\nthen we need to apply the new migration and assert that the existign data is still readable and that the schema ideally  did not change or rather taht if it did change we did not need to do a fully table rewrite.\n\nim not really sure what the right way to do that is.\n\nwe are in genreall not allowed to change the specification fo a db column in place\ni.e. if we want to expand an nvarcar field (a string) form 64 to 128\nwe ment to add a new column, migrate the data on load and remove the old colum a few release later.\n\nthe change you are makign in the modle wil not alter the size of the colume as far as i am aware based on teh sepcific values you have slected but it will chagne the interpreation and im not sure if we should there for add a new column as well in this case.","commit_id":"f41332de1b823797cb57c6085259b13326e9c3e2"},{"author":{"_account_id":34452,"name":"Joan Gilabert","display_name":"jgilaber","email":"jgilaber@redhat.com","username":"jgilaber"},"change_message_id":"ce8c15eb445aa01ef6c28f0808c764f0bcf36449","unresolved":false,"context_lines":[{"line_number":410,"context_line":"                          utils.create_test_efficacy_indicator,"},{"line_number":411,"context_line":"                          id\u003d2, uuid\u003duuid)"},{"line_number":412,"context_line":""},{"line_number":413,"context_line":"class MySQLDbEfficacyIndicatorTestCase(base.MySQLDbTestCase):"},{"line_number":414,"context_line":""},{"line_number":415,"context_line":"    FAKE_OLDER_DATE \u003d \u00272014-01-01T09:52:05.219414\u0027"},{"line_number":416,"context_line":"    FAKE_OLD_DATE \u003d \u00272015-01-01T09:52:05.219414\u0027"}],"source_content_type":"text/x-python","patch_set":5,"id":"e8ac20f8_0798e3ab","line":413,"in_reply_to":"3b47654d_45dab1ed","updated":"2025-04-15 15:10:31.000000000","message":"Acknowledged","commit_id":"f41332de1b823797cb57c6085259b13326e9c3e2"}],"watcher/tests/db/test_migrations.py":[{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"0499c0e5c3cf28fa39919520b2088e186ec83080","unresolved":true,"context_lines":[{"line_number":57,"context_line":"        self.alembic_config \u003d migration._alembic_config()"},{"line_number":58,"context_line":"        self.revisions_tested \u003d set([\"15f7375ca737\"])"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def _apply_migration(self, connection, revision):"},{"line_number":61,"context_line":"        if revision not in self.revisions_tested:"},{"line_number":62,"context_line":"            # if we don\u0027t have tests for this version, just upgrade to it"},{"line_number":63,"context_line":"            alembic_api.upgrade(self.alembic_config, revision)"}],"source_content_type":"text/x-python","patch_set":23,"id":"d27aa7fd_211acabb","line":60,"range":{"start_line":60,"start_character":8,"end_line":60,"end_character":24},"updated":"2025-05-08 18:15:46.000000000","message":"we could potentially also test the downgrade path, just checking the revision would be enough\nbut this is already a good addition to the code, nice ++","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"6f5e04f2dfec694983e705f43a10c3cd9b974308","unresolved":true,"context_lines":[{"line_number":57,"context_line":"        self.alembic_config \u003d migration._alembic_config()"},{"line_number":58,"context_line":"        self.revisions_tested \u003d set([\"15f7375ca737\"])"},{"line_number":59,"context_line":""},{"line_number":60,"context_line":"    def _apply_migration(self, connection, revision):"},{"line_number":61,"context_line":"        if revision not in self.revisions_tested:"},{"line_number":62,"context_line":"            # if we don\u0027t have tests for this version, just upgrade to it"},{"line_number":63,"context_line":"            alembic_api.upgrade(self.alembic_config, revision)"}],"source_content_type":"text/x-python","patch_set":23,"id":"06d5896f_b971ad9c","line":60,"range":{"start_line":60,"start_character":8,"end_line":60,"end_character":24},"in_reply_to":"d27aa7fd_211acabb","updated":"2025-05-09 11:16:48.000000000","message":"not needed since no downgrade option will be needed","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"0499c0e5c3cf28fa39919520b2088e186ec83080","unresolved":true,"context_lines":[{"line_number":73,"context_line":"        if post_upgrade:"},{"line_number":74,"context_line":"            post_upgrade(connection)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def _pre_upgrade_15f7375ca737(self, connection):"},{"line_number":77,"context_line":"        inspector \u003d sqlalchemy.inspect(connection)"},{"line_number":78,"context_line":"        columns \u003d inspector.get_columns(\"efficacy_indicators\")"},{"line_number":79,"context_line":"        for column in columns:"}],"source_content_type":"text/x-python","patch_set":23,"id":"0c475efb_180d6d03","line":76,"range":{"start_line":76,"start_character":8,"end_line":76,"end_character":33},"updated":"2025-05-08 18:15:46.000000000","message":"nice++","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"},{"author":{"_account_id":30002,"name":"Douglas Viroel","email":"viroel@gmail.com","username":"dviroel"},"change_message_id":"6f5e04f2dfec694983e705f43a10c3cd9b974308","unresolved":false,"context_lines":[{"line_number":73,"context_line":"        if post_upgrade:"},{"line_number":74,"context_line":"            post_upgrade(connection)"},{"line_number":75,"context_line":""},{"line_number":76,"context_line":"    def _pre_upgrade_15f7375ca737(self, connection):"},{"line_number":77,"context_line":"        inspector \u003d sqlalchemy.inspect(connection)"},{"line_number":78,"context_line":"        columns \u003d inspector.get_columns(\"efficacy_indicators\")"},{"line_number":79,"context_line":"        for column in columns:"}],"source_content_type":"text/x-python","patch_set":23,"id":"a9659f91_18fea53f","line":76,"range":{"start_line":76,"start_character":8,"end_line":76,"end_character":33},"in_reply_to":"0c475efb_180d6d03","updated":"2025-05-09 11:16:48.000000000","message":"Done","commit_id":"0ed3d4de83cf241649d2448d0b29aced371e6355"}]}