Skip to content

Commit d54a840

Browse files
SantanMhengxian-jiangCopilot
authored
add http tests for the study and its flow run use case (#2691)
* Use tokenStudyCode instead of studyId in strategus flow * Update flows to use tokenStudyCode * Remove unintended changes * Adapt tokenStudyCode on functions side * More align in tokenStudyCode * Verify and fix * add strategus http tests, with hardcoded treatment patterns specification * Add strategus analysis integration test as a separate daily scheduled GHA workflow * Fix workflow branch trigger to use current branch for testing * Add Prefect diagnostics on strategus flow run failure in CI * Add Prefect diagnostics on strategus flow run failure in CI * fix env in gh action * fix env in gh action * add a patch for treatment patterns for this version * change databaseconnector remote ref * add flow-hades build; to revert * use cache schema name instead hardcoded schemaname * concat databasename and schemaname * add print logs, to revert * use cache_id in DBDao * add cache_id placeholder and in active places * fix trexdao * fix strategus DuckDB schema qualification * align strategus results cache routing * Fix Strategus execution schema qualification (#2832) Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> * WIP, todo: revert * use cdmsettings for schemaname * successful flow execution on local * revert building docker image * reorder steps in gh action * add a comment * fixes for results persistence * add missing env * add back the env * use _shared env * add default value to the flow * fix copilot comment * change sqlrender to alp-dqd branch * remove branch in GHA on event * use env Signed-off-by: Santan Maddi <55120902+SantanM@users.noreply.github.com> --------- Signed-off-by: Santan Maddi <55120902+SantanM@users.noreply.github.com> Co-authored-by: hengxian-jiang <hengxian.jiang@u.nus.edu> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
1 parent ac58825 commit d54a840

15 files changed

Lines changed: 814 additions & 36 deletions

File tree

.github/workflows/_test-http-duckdb.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -377,7 +377,7 @@ jobs:
377377
echo "isTestEnv: ${isTestEnv:-false}"
378378
echo "TESTSCHEMA: ${TESTSCHEMA}"
379379
echo "HTTPTEST_DB_DIALECT: ${HTTPTEST_DB_DIALECT}"
380-
yarn test-specs --test_schema_name=$TESTSCHEMA
380+
yarn test-specs --test_schema_name=$TESTSCHEMA '--file_pattern=^(?!strategus_analysis_test).*_test\.js$'
381381
382382
- name: Mark HTTP duckdb attempt success
383383
if: success()

.github/workflows/strategus-analysis-integration-test.yml

Lines changed: 441 additions & 0 deletions
Large diffs are not rendered by default.

README.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
[![Commuity](https://img.shields.io/badge/Community-Slack-d9dbec)](https://join.slack.com/t/data2evidence/shared_invite/zt-3vabnh2qr-vMev2VfLI2Sl1YA27gGVig)
1616
[![GitHub Activity](https://img.shields.io/github.qkg1.topmit-activity/m/ohdsi/d2e?label=Commit%20activty&logo=github&color=d9dbec)](https://github.com/ohdsi/d2e/graphs/contributors)
1717
[![D2E CLI Version](https://img.shields.io/npm/v/d2e?label=D2E%20installer&logo=npm&color=d9dbec)](https://www.npmjs.com/package/d2e)
18+
[![Strategus Analysis Tests](https://github.com/ohdsi/d2e/actions/workflows/strategus-analysis-integration-test.yml/badge.svg?branch=develop)](https://github.com/ohdsi/d2e/actions/workflows/strategus-analysis-integration-test.yml)
1819

1920
## Overview
2021

plugins/flows/_shared_flow_utils/dao/trexdao.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -484,7 +484,9 @@ def get_database_connector_connection_string(self) -> str:
484484
host = self.tenant_configs.host
485485
port = self.tenant_configs.port
486486

487-
return f"{DialectDrivers.jdbc.trex}://{host}:{port}/{self.database_code}?preferQueryMode=simple&autocommit=true"
487+
# Match Python's _get_connection: connect on cache_id so unqualified queries resolve there.
488+
jdbc_dbname = self.cache_id or self.database_code
489+
return f"{DialectDrivers.jdbc.trex}://{host}:{port}/{jdbc_dbname}?preferQueryMode=simple&autocommit=true"
488490

489491
def get_database_connector_dbms_val(self) -> str:
490492
return DialectDrivers.database_connector.trex

plugins/flows/hades/renv.lock

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -934,7 +934,7 @@
934934
"RemoteHost": "api.github.com",
935935
"RemoteUsername": "data2evidence",
936936
"RemoteRepo": "d2e-DatabaseConnector",
937-
"RemoteRef": "v6.3.2-0.0.1"
937+
"RemoteRef": "alp-dqd"
938938
},
939939
"DeepPatientLevelPrediction": {
940940
"Package": "DeepPatientLevelPrediction",
@@ -2699,8 +2699,7 @@
26992699
"RemoteHost": "api.github.com",
27002700
"RemoteUsername": "data2evidence",
27012701
"RemoteRepo": "d2e-SqlRender",
2702-
"RemoteRef": "6573c8044a9ddc27da5222a50dc4d9ecdbc59525",
2703-
"RemoteSha": "6573c8044a9ddc27da5222a50dc4d9ecdbc59525"
2702+
"RemoteRef": "alp-dqd"
27042703
},
27052704
"TTR": {
27062705
"Package": "TTR",

plugins/flows/hades/strategus_plugin/dc-local.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,9 @@
1-
# docker compose -f flows/hades/strategus_plugin/dc-local.yml up -d
1+
# docker compose -f ./plugins/flows/hades/strategus_plugin/dc-local.yml up -d
22
services:
33
app:
44
platform: linux/amd64
55
image: ghcr.io/ohdsi/d2e/flow-hades:develop
6+
# entrypoint: tail -f /dev/null
67
# with debugger listening
78
# entrypoint: python -m debugpy --listen 0.0.0.0:9234 --wait-for-client -m flows.hades.strategus_plugin.flow
89
entrypoint: /app/.venv/bin/python -m pytest /app/flows/hades/strategus_plugin/tests/

plugins/flows/hades/strategus_plugin/flow.py

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
from prefect import flow, task
88
from prefect.context import TaskRunContext, FlowRunContext, get_run_context
9+
from prefect.variables import Variable
910
from prefect.artifacts import create_markdown_artifact
1011

1112
from .hooks import generate_nodes_flow_hook, execute_nodes_flow_hook, node_task_execution_hook
@@ -53,6 +54,7 @@ def strategus_plugin(json_graph, options):
5354
update_results_schema = _options.get('updateResultsSchema', True)
5455
databaseCode = options.get('databaseCode', None)
5556
datasetId = options.get('datasetId', None)
57+
cacheId = options.get('cacheId', None)
5658
studyName = options.get("studyName", "")
5759
tokenStudyCode = options.get('tokenStudyCode', None)
5860

@@ -91,11 +93,18 @@ def strategus_plugin(json_graph, options):
9193
)
9294
# updateResultsSchema option will drop the existing schema before uploading new results
9395
if(update_results_schema):
94-
drop_strategus_results(options)
96+
results_db_settings = {
97+
'databaseCode': Variable.get('trex_strategus_results_db_name', 'strategus_results'),
98+
'cacheId': cacheId,
99+
"datasetID": datasetId,
100+
"tokenStudyCode": tokenStudyCode
101+
}
102+
drop_strategus_results(results_db_settings)
95103

96104
if(upload_results):
97105
result_db_settings = {
98-
'database_code': databaseCode,
106+
'database_code': Variable.get('trex_strategus_results_db_name', 'strategus_results'),
107+
'cache_id': cacheId,
99108
"dataset_id": datasetId,
100109
"token_study_code": tokenStudyCode
101110
}
@@ -215,6 +224,7 @@ def runStrategus(json_graph, options):
215224
token_study_code = options.get('tokenStudyCode', None)
216225
datasetId = options.get('datasetId', None)
217226
database_code = options.get('databaseCode', None)
227+
cache_id = options.get('cacheId', None)
218228
schema_name = options.get('schemaName', None)
219229
upload_results = options.get('uploadResults', False)
220230
update_results_schema = options.get('updateResultsSchema', True)
@@ -228,7 +238,7 @@ def runStrategus(json_graph, options):
228238
if(not schema_name):
229239
raise Exception('Schema name is missing')
230240

231-
dbSettings = { "database_code": database_code, "schema_name": schema_name, "dataset_id": datasetId, "token_study_code": token_study_code }
241+
dbSettings = { "database_code": database_code, "cache_id": cache_id, "schema_name": schema_name, "dataset_id": datasetId, "token_study_code": token_study_code }
232242
base_path = f'/tmp/{flow_run_id}'
233243
work_folder = f'{base_path}/work'
234244
path_to_results = f'{base_path}/results'
@@ -243,6 +253,8 @@ def runStrategus(json_graph, options):
243253

244254
analysisSpec = json.dumps(analysisSpec)
245255
defaultExecutionSettings = getRCdmExecutionSettings({
256+
"cacheId": cache_id,
257+
"dbName": cache_id if cache_id else database_code,
246258
"schemaName": schema_name,
247259
"workFolder": work_folder,
248260
"resultsFolder": path_to_results
@@ -252,11 +264,16 @@ def runStrategus(json_graph, options):
252264
execute_r_strategus(analysisSpec, executionSettings, dbSettings)
253265
# updateResultsSchema option will drop the existing schema before uploading new results
254266
if(update_results_schema):
255-
drop_strategus_results(options)
267+
drop_strategus_results({
268+
'databaseCode': Variable.get('trex_strategus_results_db_name', 'strategus_results'),
269+
'cacheId': cache_id,
270+
'tokenStudyCode': token_study_code
271+
})
256272

257273
if(upload_results):
258274
result_db_settings = {
259-
'database_code': database_code,
275+
'database_code': Variable.get('trex_strategus_results_db_name', 'strategus_results'),
276+
'cache_id': cache_id,
260277
"dataset_id": datasetId,
261278
"token_study_code": token_study_code
262279
}
@@ -297,6 +314,7 @@ def drop_strategus_results(options):
297314

298315
drop_strategus_results_schema(dbSettings={
299316
'database_code': database_code,
317+
'cache_id': options.get('cacheId', None),
300318
'token_study_code': token_study_code
301319
})
302320

plugins/flows/hades/strategus_plugin/nodes.py

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1186,10 +1186,16 @@ def task(self, nodes, results, task_run_context):
11861186

11871187
databaseConnectorJarFolder = '/app/inst/drivers'
11881188
os.environ['DATABASECONNECTOR_JAR_FOLDER'] = databaseConnectorJarFolder
1189-
dbSettings = { "database_code": self.flowOptions["databaseCode"], "schema_name": self.flowOptions["schemaName"], "dataset_id": self.flowOptions["datasetId"] }
1189+
dbSettings = {
1190+
"database_code": self.flowOptions["databaseCode"],
1191+
"cache_id": self.flowOptions.get("cacheId"),
1192+
"schema_name": self.flowOptions["schemaName"],
1193+
"dataset_id": self.flowOptions["datasetId"],
1194+
}
11901195
dbdao = DBDao(
11911196
dialect=SupportedDatabaseDialects.TREX if USE_TREX_CONNECTION else None,
1192-
database_code=dbSettings['database_code']
1197+
database_code=dbSettings['database_code'],
1198+
cache_id=dbSettings.get('cache_id', None)
11931199
)
11941200
db_credentials = dbdao.tenant_configs
11951201
rDatabaseConnector = ro.packages.importr('DatabaseConnector')
@@ -1207,6 +1213,7 @@ def task(self, nodes, results, task_run_context):
12071213
path_to_results = f'{base_path}/results'
12081214

12091215
executionSettings = getRCdmExecutionSettings({
1216+
"cacheId": dbSettings.get("cache_id"),
12101217
"schemaName": dbSettings['schema_name'],
12111218
"workFolder": work_folder,
12121219
"resultsFolder": path_to_results
@@ -1241,22 +1248,27 @@ def task(self, nodes, results, task_run_context):
12411248
def get_strategus_node(options):
12421249
return StrategusNode({"id": str(uuid.uuid4()), "type": "strategus_node", "flowOptions": options})
12431250

1251+
12441252
@flow(name="execute-r-strategus",
12451253
log_prints=True)
12461254
def execute_r_strategus(analysisSpec: str, executionSettings, dbSettings):
12471255
with ro.default_converter.context():
12481256
try:
12491257
ro.r(set_trex_env_var(USE_TREX_CONNECTION))
12501258
database_code = dbSettings['database_code']
1259+
cache_id = dbSettings.get('cache_id', None)
12511260
rParallelLogger = importr('ParallelLogger')
12521261
rDatabaseConnector = importr('DatabaseConnector')
12531262
databaseConnectorJarFolder = '/app/inst/drivers'
12541263

12551264
dbdao = DBDao(
12561265
dialect=SupportedDatabaseDialects.TREX if USE_TREX_CONNECTION else None,
1257-
database_code=database_code
1266+
database_code=database_code,
1267+
cache_id=cache_id
12581268
)
12591269
db_credentials = dbdao.tenant_configs
1270+
print(f"Connecting to database with connection string: {dbdao.get_database_connector_connection_string()}")
1271+
12601272
rConnectionDetails = rDatabaseConnector.createConnectionDetails(
12611273
dbms=dbdao.get_database_connector_dbms_val(),
12621274
connectionString=dbdao.get_database_connector_connection_string(),
@@ -1296,6 +1308,9 @@ def execute_r_strategus(analysisSpec: str, executionSettings, dbSettings):
12961308
def execute(rSpec, rExecutionSettings, rConnectionDetails):
12971309
with ro.default_converter.context():
12981310
ro.r(set_trex_env_var(USE_TREX_CONNECTION))
1311+
_patch = "/app/flows/hades/strategus_plugin/patch_treatment_patterns.R"
1312+
if os.path.exists(_patch):
1313+
ro.r(f'source("{_patch}")')
12991314
ro.r('cat("Max Java Heap Size (GB): ", .jcall(.jnew("java/lang/Runtime"), "J", "maxMemory") / 1e9, "\\n")')
13001315
rStrategus = importr('Strategus')
13011316
rStrategus.execute(connectionDetails = rConnectionDetails, analysisSpecifications = rSpec, executionSettings = rExecutionSettings)
@@ -1307,7 +1322,7 @@ def upload_strategus_results(analysisSpec: str, path_to_results, dbSettings):
13071322
try:
13081323
ro.r(set_trex_env_var(USE_TREX_CONNECTION))
13091324
database_code = dbSettings['database_code']
1310-
results_schema = f'results_{validate_token_study_code(dbSettings["token_study_code"])}'
1325+
results_schema = f'{database_code}.results_{validate_token_study_code(dbSettings["token_study_code"])}'
13111326
rStrategus = importr('Strategus')
13121327
rParallelLogger = importr('ParallelLogger')
13131328
rDatabaseConnector = importr('DatabaseConnector')
@@ -1353,6 +1368,8 @@ def upload_strategus_results(analysisSpec: str, path_to_results, dbSettings):
13531368
);
13541369
"""
13551370
dbdao.execute_sql(create_table_sql)
1371+
else:
1372+
print(f'Schema {results_schema} already exists, skipping creation of results datamodel')
13561373

13571374
# uploadResults logs are not captured by default
13581375
# so we override the consolewrite_print callback to capture the logs
@@ -1431,8 +1448,8 @@ def getRCdmExecutionSettings(settings) -> str:
14311448
rCohortGenerator = importr('CohortGenerator')
14321449

14331450
rExecutionSettings = rStrategus.createCdmExecutionSettings(
1434-
workDatabaseSchema = settings['schemaName'],
1435-
cdmDatabaseSchema = settings['schemaName'],
1451+
workDatabaseSchema = f"{settings['dbName']}.{settings['schemaName']}",
1452+
cdmDatabaseSchema = f"{settings['dbName']}.{settings['schemaName']}",
14361453
cohortTableNames = rCohortGenerator.getCohortTableNames(cohortTable = "cohort"),
14371454
workFolder = settings['workFolder'],
14381455
resultsFolder = settings['resultsFolder'],
Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
local({
2+
ns <- getNamespace("TreatmentPatterns")
3+
cls <- get("CDMInterface", envir = ns)
4+
5+
patched <- function(cohorts, cohortTableName, andromeda, andromedaTableName, minEraDuration) {
6+
targetCohortId <- getCohortIds(cohorts, "target")
7+
n <- unlist(lapply(cohortTableName, function(cohortTable) {
8+
as.numeric(unlist(DatabaseConnector::renderTranslateQuerySql(
9+
connection = private$.connection,
10+
sql = "\n SELECT COUNT(*)\n FROM @resultSchema.@cohortTable\n WHERE cohort_definition_id IN (@cohortIds)\n GROUP BY subject_id;",
11+
resultSchema = private$.resultSchema, cohortTable = cohortTable, cohortIds = cohorts$cohortId
12+
)))
13+
}))
14+
private$dbAppendAttrition(n, andromeda, sort(cohorts$cohortId))
15+
16+
sql <- lapply(cohortTableName, function(tableName) {
17+
SqlRender::loadRenderTranslateSql(
18+
sqlFilename = "selectData.sql", packageName = "TreatmentPatterns",
19+
dbms = private$.connection@dbms, tempEmulationSchema = private$.tempEmulationSchema,
20+
resultSchema = private$.resultSchema, cdmSchema = private$.cdmSchema,
21+
cohortTable = tableName, cohortIds = cohorts$cohortId, minEraDuration = minEraDuration
22+
)
23+
})
24+
25+
# FIX: loadRenderTranslateSql appends a trailing semicolon. Embedding it directly
26+
# into sprintf("FROM (\n %s\n) a") places the semicolon inside the subquery, which
27+
# causes splitSql to split mid-statement producing an incomplete CREATE TEMP TABLE.
28+
sql <- lapply(sql, function(s) trimws(sub(";[[:space:]]*$", "", s)))
29+
30+
renderedSql <- paste(sql, collapse = "\nUNION ALL\n")
31+
DatabaseConnector::renderTranslateExecuteSql(
32+
connection = private$.connection, oracleTempSchema = private$.tempEmulationSchema,
33+
sql = sprintf("DROP TABLE IF EXISTS #tp_dbc_cohort_table;\n\n SELECT *\n INTO #tp_dbc_cohort_table\n FROM (\n %s\n ) a;", renderedSql),
34+
tempEmulationSchema = private$.tempEmulationSchema
35+
)
36+
37+
DatabaseConnector::renderTranslateQuerySqlToAndromeda(
38+
connection = private$.connection, andromeda = andromeda,
39+
andromedaTableName = andromedaTableName, tempEmulationSchema = private$.tempEmulationSchema,
40+
sql = "\n SELECT\n #tp_dbc_cohort_table.cohort_definition_id AS cohort_definition_id,\n #tp_dbc_cohort_table.subject_id AS subject_id,\n #tp_dbc_cohort_table.cohort_start_date,\n #tp_dbc_cohort_table.cohort_end_date,\n #tp_dbc_cohort_table.age,\n #tp_dbc_cohort_table.sex,\n #tp_dbc_cohort_table.subject_id_origin\n FROM #tp_dbc_cohort_table\n INNER JOIN (\n SELECT #tp_dbc_cohort_table.subject_id\n FROM #tp_dbc_cohort_table\n WHERE #tp_dbc_cohort_table.cohort_definition_id IN (@targetCohortId)\n ) AS cross_sec\n ON cross_sec.subject_id = #tp_dbc_cohort_table.subject_id",
41+
targetCohortId = targetCohortId
42+
)
43+
44+
names(andromeda[[andromedaTableName]]) <- tolower(names(andromeda[[andromedaTableName]]))
45+
if (inherits(private$.connection, "DatabaseConnectorJdbcConnection") &&
46+
private$.connection@dbms == "postgresql") {
47+
andromeda[[andromedaTableName]] <- andromeda[[andromedaTableName]] %>%
48+
dplyr::mutate(
49+
cohort_start_date = dplyr::sql("datediff('day', DATE '1970-01-01', cohort_start_date)"),
50+
cohort_end_date = dplyr::sql("datediff('day', DATE '1970-01-01', cohort_end_date)")
51+
)
52+
} else {
53+
andromeda[[andromedaTableName]] <- andromeda[[andromedaTableName]] %>%
54+
dplyr::mutate(
55+
cohort_start_date = as.integer(.data$cohort_start_date),
56+
cohort_end_date = as.integer(.data$cohort_end_date)
57+
)
58+
}
59+
60+
n <- andromeda[[andromedaTableName]] %>%
61+
dplyr::group_by(.data$subject_id) %>% dplyr::summarise(n = dplyr::n()) %>% dplyr::pull()
62+
appendAttrition(
63+
toAdd = data.frame(number_records = sum(n), number_subjects = length(n), reason_id = 2,
64+
reason = sprintf("Removing records < minEraDuration (%s)", minEraDuration),
65+
time_stamp = as.numeric(Sys.time())),
66+
andromeda = andromeda
67+
)
68+
return(andromeda)
69+
}
70+
71+
cls$set("private", "dbconFetchCohortTable", patched, overwrite = TRUE)
72+
message("Patched TreatmentPatterns::CDMInterface$dbconFetchCohortTable (trailing semicolon fix)")
73+
})

plugins/functions/alp-dataflow-gen-init/src/env.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ export const env = {
4242
trex_sql_port: _env.TREX__SQL__PORT,
4343
trex_sql_dbname: _env.TREX__SQL__DBNAME,
4444
trex_sql_user: _env.TREX__SQL__USER,
45+
trex_strategus_results_db_name: _env.TREX__STRATEGUS_RESULTS_DB_NAME || "strategus_results",
4546
is_dev_env: _env.PREFECT__LOCAL_DEBUG === "true" || false, // Default to false if not set
4647
logs_debug_enable: _env.PREFECT__LOGS_DEBUG_ENABLED === "true" || false, // Default to false if not set
4748
fhir_database_code: _env.FHIR_DATABASE_CODE,

0 commit comments

Comments
 (0)