Skip to content

Commit 98ec5ba

Browse files
committed
misc: Code review feedback
1 parent 09c7214 commit 98ec5ba

6 files changed

Lines changed: 22 additions & 21 deletions

File tree

testgen/__main__.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@
4646
from testgen.common.models.profiling_run import ProfilingRun
4747
from testgen.common.models.test_run import TestRun
4848
from testgen.common.models.test_suite import TestSuite
49+
from testgen.common.notifications.profiling_run import send_profiling_run_notifications
4950
from testgen.common.notifications.test_run import send_test_run_notifications
5051
from testgen.scheduler import register_scheduler_job, run_scheduler
5152
from testgen.utils import plugins
@@ -648,7 +649,8 @@ def run_ui():
648649
@with_database_session
649650
def cancel_all_running():
650651
try:
651-
ProfilingRun.cancel_all_running()
652+
for profiling_run_id in ProfilingRun.cancel_all_running():
653+
send_profiling_run_notifications(ProfilingRun.get(profiling_run_id))
652654
for test_run_id in TestRun.cancel_all_running():
653655
send_test_run_notifications(TestRun.get(test_run_id))
654656
except Exception:

testgen/common/models/hygiene_issue.py

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -107,11 +107,10 @@ def select_with_diff(
107107
order_weight = case(
108108
(cls.priority == "Definite", 1),
109109
(cls.priority == "Likely", 2),
110-
(cls.priority == "Possible", 5),
111-
(cls.priority == "High", 2),
112-
(cls.priority == "Moderate", 4),
113-
(cls.priority == "Low", 6),
114-
else_=7,
110+
(cls.priority == "Possible", 3),
111+
(cls.priority == "High", 4),
112+
(cls.priority == "Moderate", 5),
113+
else_=6,
115114
)
116115
is_new_col = (other.id.is_(None) if other_profiling_run_id else null()).label("is_new")
117116
query = (

testgen/common/models/profiling_run.py

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -237,14 +237,18 @@ def has_running_process(cls, ids: list[str]) -> bool:
237237
return process_count > 0
238238

239239
@classmethod
240-
def cancel_all_running(cls) -> None:
240+
def cancel_all_running(cls) -> list[UUID]:
241241
query = (
242-
update(cls).where(cls.status == "Running").values(status="Cancelled", profiling_endtime=datetime.now(UTC))
242+
update(cls)
243+
.where(cls.status == "Running")
244+
.values(status="Cancelled", profiling_endtime=datetime.now(UTC))
245+
.returning(cls.id)
243246
)
244247
db_session = get_current_session()
245-
db_session.execute(query)
248+
rows = db_session.execute(query)
246249
db_session.commit()
247250
cls.clear_cache()
251+
return [r.id for r in rows]
248252

249253
@classmethod
250254
def cancel_run(cls, run_id: str | UUID) -> None:

testgen/common/notifications/profiling_run.py

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ class ProfilingRunEmailTemplate(BaseNotificationTemplate):
2424
def get_subject_template(self) -> str:
2525
return (
2626
"[TestGen] Profiling Run {{format_status profiling_run.status}}: {{table_groups_name}}"
27-
"{{#if new_issue_count}}"
28-
' | {{format_number new_issue_count}} new hygiene {{pluralize new_issue_count "issue" "issues"}}'
27+
"{{#if issue_count}}"
28+
' | {{format_number issue_count}} hygiene {{pluralize issue_count "issue" "issues"}}'
2929
"{{/if}}"
3030
)
3131

@@ -313,8 +313,7 @@ def send_profiling_run_notifications(profiling_run: ProfilingRun, result_list_ct
313313
"table_ct": profiling_run.table_ct,
314314
"column_ct": profiling_run.column_ct,
315315
},
316-
"issue_count": len(issues),
317-
"new_issue_count": sum(1 for _, is_new in issues if is_new),
316+
"issue_count": sum(c.total for c in counts.values()),
318317
"hygiene_issues_summary": hygiene_issues_summary,
319318
**dict(get_current_session().execute(labels_query).one()),
320319
}

testgen/ui/views/profiling_runs.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
from testgen.common.models.project import Project
1717
from testgen.common.models.scheduler import RUN_PROFILE_JOB_KEY
1818
from testgen.common.models.table_group import TableGroup, TableGroupMinimal
19+
from testgen.common.notifications.profiling_run import send_profiling_run_notifications
1920
from testgen.ui.components import widgets as testgen
2021
from testgen.ui.components.widgets import testgen_component
2122
from testgen.ui.navigation.menu import MenuItem
@@ -169,6 +170,7 @@ def on_cancel_run(profiling_run: dict) -> None:
169170
process_status, process_message = process_service.kill_profile_run(to_int(profiling_run["process_id"]))
170171
if process_status:
171172
ProfilingRun.cancel_run(profiling_run["id"])
173+
send_profiling_run_notifications(ProfilingRun.get(profiling_run["id"]))
172174

173175
fm.reset_post_updates(str_message=f":{'green' if process_status else 'red'}[{process_message}]", as_toast=True)
174176

@@ -218,6 +220,7 @@ def on_delete_confirmed(*_args) -> None:
218220
process_status, _ = process_service.kill_profile_run(to_int(profiling_run.process_id))
219221
if process_status:
220222
ProfilingRun.cancel_run(profiling_run.id)
223+
send_profiling_run_notifications(ProfilingRun.get(profiling_run.id))
221224
ProfilingRun.cascade_delete(profiling_run_ids)
222225
st.rerun()
223226
except Exception:

tests/unit/test_score_drop_notifications.py

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -48,12 +48,6 @@ def send_mock():
4848
yield mock
4949

5050

51-
@pytest.fixture
52-
def db_session_mock():
53-
with patch("testgen.common.notifications.score_drop.get_current_session") as mock:
54-
yield mock
55-
56-
5751
@pytest.fixture
5852
def select_mock():
5953
with patch("testgen.common.notifications.score_drop.select") as mock:
@@ -127,7 +121,7 @@ def test_send_score_drop_notifications_no_match(
127121
]
128122
for ns in ns_select_result:
129123
ns.score_definition_id = "sd-x"
130-
db_session_mock().scalars.return_value = ns_select_result
124+
db_session_mock.execute().fetchall.return_value = ns_select_result
131125

132126
send_score_drop_notifications(data)
133127

@@ -157,7 +151,7 @@ def test_send_score_drop_notifications(
157151
(score_definition, "score", total_prev, total_fresh),
158152
(score_definition, "cde_score", cde_prev, cde_fresh)
159153
]
160-
db_session_mock().execute().fetchall.return_value = [(ns, "Test Project") for ns in ns_select_result]
154+
db_session_mock.execute().fetchall.return_value = [(ns, "Test Project") for ns in ns_select_result]
161155

162156
send_score_drop_notifications(data)
163157

0 commit comments

Comments
 (0)