EHDEN / CdmInspection

R Package to support quality control inspection of an OMOP-CDM instance
Apache License 2.0
11 stars 16 forks source link

Error object 'databaseName' not found #47

Closed sdebruyn closed 3 years ago

sdebruyn commented 3 years ago

Running on an Azure SQL Database from R Studio on macOS:

I also tried this without prefixing the database name (lynxcare) to the schema name and got the same result.

> CdmInspection::cdmInspection(connectionDetails, "lynxcare.omop_v5", "lynxcare.omop_dq", "lynxcare.omop_v5", runWebAPIChecks = FALSE)
Connecting using SQL Server driver
Connecting using SQL Server driver
CDM Inspection of database LynxCare-5eu7a29znihoniv9w started (cdm_version=5.3.1)
Running Data Table Checks
Connecting using SQL Server driver
> Failed see output/data_tables_countErr.txt for more details
Connecting using SQL Server driver
> Failed see output/totalrecordsErr.txt for more details
Connecting using SQL Server driver
> Failed see output/recordsperpersonErr.txt for more details
Connecting using SQL Server driver
> Failed see output/conceptsperpersonErr.txt for more details
Connecting using SQL Server driver
> Vocabulary table successfully extracted
Running Vocabulary Checks
Connecting using SQL Server driver
>  Mapping Completeness query executed successfully in 0.40 secs
Connecting using SQL Server driver
>  Drug Level Mapping query executed successfully in 0.30 secs
Connecting using SQL Server driver
>  Unmapped drugs query executed successfully in 0.30 secs
Connecting using SQL Server driver
>  Unmapped conditions query executed successfully in 0.32 secs
Connecting using SQL Server driver
>  Unmapped measurements query executed successfully in 0.31 secs
Connecting using SQL Server driver
>  Unmapped observations query executed successfully in 0.29 secs
Connecting using SQL Server driver
>  Unmapped procedures query executed successfully in 0.31 secs
Connecting using SQL Server driver
>  Unmapped devices query executed successfully in 0.31 secs
Connecting using SQL Server driver
>  Mapped drugs query executed successfully in 0.31 secs
Connecting using SQL Server driver
>  Mapped conditions query executed successfully in 0.28 secs
Connecting using SQL Server driver
>  Mapped measurements query executed successfully in 0.33 secs
Connecting using SQL Server driver
>  Mapped observations query executed successfully in 0.29 secs
Connecting using SQL Server driver
>  Mapped procedures query executed successfully in 0.29 secs
Connecting using SQL Server driver
>  Mapped devices query executed successfully in 0.32 secs
Connecting using SQL Server driver
>  Concept counts by vocabulary query executed successfully in 0.98 secs
Connecting using SQL Server driver
>  Count on vocabulary tables query executed successfully in 8.65 secs
Connecting using SQL Server driver
>  Source to concept map breakdown query executed successfully in 0.40 secs
Connecting using SQL Server driver
>  Source to concept map query executed successfully in 21.36 secs
Check installed R Packages
> All HADES packages are installed
Running Performance Checks on Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz cpu with 6 cores, and 6.24 GB ram.
Running Performance Checks SQL
Connecting using SQL Server driver
> Failed see output/achilles_timingErr.txt for more details
Connecting using SQL Server driver
>  Executing vocabulary query benchmark in 4.63 secs
Done.
Complete CdmInspection took  0.71  minutes
Error in CdmInspection::cdmInspection(connectionDetails, "lynxcare.omop_v5",  : 
  object 'databaseName' not found
PRijnbeek commented 3 years ago

Could you share an error.txt file as shown in the log.

You need to name the parameters in your call to the package as shown in the instructions:

results<-cdmInspection(connectionDetails, cdmDatabaseSchema = cdmDatabaseSchema, resultsDatabaseSchema = resultsDatabaseSchema, vocabDatabaseSchema = vocabDatabaseSchema, oracleTempSchema = oracleTempSchema, sourceName = databaseName, runVocabularyChecks = TRUE, runDataTablesChecks = TRUE, runPerformanceChecks = TRUE, runWebAPIChecks = TRUE, smallCellCount = smallCellCount, baseUrl = baseUrl, sqlOnly = FALSE, outputFolder = outputFolder, verboseMode = verboseMode)

PRijnbeek commented 3 years ago

I think it is not an issue if you follow the instructions since then databaseName is global, but it is better if we change sourceName to databaseName in the call. Will fix this tonight and merge all updates into a new release.

PRijnbeek commented 3 years ago

Fixed in latest version

sdebruyn commented 3 years ago

@PRijnbeek The issue was not fixed by explicitly naming the arguments. Also, there is no argument named databaseName... log_cdmInspection (1).txt

CdmInspection::cdmInspection(connectionDetails = connectionDetails,
                             cdmDatabaseSchema = "dbname.schemaname",
                             resultsDatabaseSchema = "dbname.schemaname",
                             vocabDatabaseSchema = "dbname.schemaname",
                             sourceName = "private",
                             outputFolder = "/tmp/omop/cdm_inspection",
                             runWebAPIChecks = FALSE,
                             verboseMode = TRUE)
Connecting using SQL Server driver
CDM Inspection of database lynxcare_nlp_PRIVATE started (cdm_version=5.3.1)
Running Data Table Checks
Connecting using SQL Server driver
>  Data tables count query executed successfully in 0.39 secs
Connecting using SQL Server driver
>  Total number of records over time query executed successfully in 0.28 secs
Connecting using SQL Server driver
>  Number of records per person query executed successfully in 0.21 secs
Connecting using SQL Server driver
>  Number of records per person query executed successfully in 0.19 secs
Connecting using SQL Server driver
> Vocabulary table successfully extracted
Running Vocabulary Checks
Connecting using SQL Server driver
>  Mapping Completeness query executed successfully in 0.24 secs
Connecting using SQL Server driver
>  Drug Level Mapping query executed successfully in 0.18 secs
Connecting using SQL Server driver
>  Unmapped drugs query executed successfully in 0.09 secs
Connecting using SQL Server driver
>  Unmapped conditions query executed successfully in 0.13 secs
Connecting using SQL Server driver
>  Unmapped measurements query executed successfully in 0.22 secs
Connecting using SQL Server driver
>  Unmapped observations query executed successfully in 0.20 secs
Connecting using SQL Server driver
>  Unmapped procedures query executed successfully in 0.12 secs
Connecting using SQL Server driver
>  Unmapped devices query executed successfully in 0.10 secs
Connecting using SQL Server driver
>  Mapped drugs query executed successfully in 0.11 secs
Connecting using SQL Server driver
>  Mapped conditions query executed successfully in 0.18 secs
Connecting using SQL Server driver
>  Mapped measurements query executed successfully in 0.10 secs
Connecting using SQL Server driver
>  Mapped observations query executed successfully in 0.12 secs
Connecting using SQL Server driver
>  Mapped procedures query executed successfully in 0.27 secs
Connecting using SQL Server driver
>  Mapped devices query executed successfully in 0.22 secs
Connecting using SQL Server driver
>  Concept counts by vocabulary query executed successfully in 0.75 secs
Connecting using SQL Server driver
>  Count on vocabulary tables query executed successfully in 12.15 secs
Connecting using SQL Server driver
>  Source to concept map breakdown query executed successfully in 0.36 secs
Connecting using SQL Server driver
>  Source to concept map query executed successfully in 24.89 secs
Check installed R Packages
> All HADES packages are installed
Loading required package: benchmarkme
Running Performance Checks on Intel(R) Xeon(R) CPU E5-2673 v3 @ 2.40GHz cpu with 4 cores, and 16.82 GB ram.
Running Performance Checks SQL
Connecting using SQL Server driver
>  Retrieving duration of Achilles queries in 0.11 secs
Connecting using SQL Server driver
>  Executing vocabulary query benchmark in 5.05 secs
Done.
Complete CdmInspection took  0.91  minutes
Error in CdmInspection::cdmInspection(connectionDetails = connectionDetails,  :
  object 'databaseName' not found
sdebruyn commented 3 years ago

Also, there is no new version, latest release is still 1.0.1?

PRijnbeek commented 3 years ago

@sdebruyn you need to re-open the issue first. I keep it closed because master was updated with the fix just not an official release yet which is added now (note that if you would had updated the package it would have worked).

Re-open the issue if you still have a problem. Note the change in the CodeToRun file with the databaseName variable.

sdebruyn commented 3 years ago

I cannot reopen closed issues since I'm not a maintainer of this project.

We don't install from the master branch, we want a more controlled environment, so I specify the tag manually.

PRijnbeek commented 3 years ago

Sure I understand. I will re-open it and you can close when solved.

sdebruyn commented 3 years ago

I'm still getting the same error with the newer version

This is what I run:

CdmInspection::cdmInspection(connectionDetails = connectionDetails,
                             cdmDatabaseSchema = sprintf("%s.%s", Sys.getenv("OMOP_DB_NAME"), Sys.getenv("OMOP_DB_SCHEMA", unset="omop_v5")),
                             resultsDatabaseSchema = sprintf("%s.omop_dq", Sys.getenv("OMOP_DB_NAME"), Sys.getenv("OMOP_DB_RESULTS", unset="omop_dq")),
                             vocabDatabaseSchema = sprintf("%s.%s", Sys.getenv("OMOP_DB_NAME"), Sys.getenv("OMOP_DB_SCHEMA", unset="omop_v5")),
                             databaseName = Sys.getenv("OMOP_DB_NAME"),
                             outputFolder = "/tmp/omop/cdm_inspection",
                             runWebAPIChecks = FALSE,
                             verboseMode = Sys.getenv("VERBOSE_MODE", unset="FALSE"))

All the mentioned env vars are set

PRijnbeek commented 3 years ago

Do not understand this . I have not problems running it on my side so cannot reproduce your error. Also at least 8 other databases have executed it. Can you clone the package and build it and share where the error occurs in the code.

sdebruyn commented 3 years ago

As you can see in the original issue above, the variable databaseName/sourceName was always set as you can see the log output from this line: https://github.com/EHDEN/CdmInspection/blob/master/R/CdmInspection.R#L114

PRijnbeek commented 3 years ago

Exactly that is why I do not understand your error. As mentioned above, build the package look at error trace, or place a breakpoint and step through it. We can do that together if needed.

sdebruyn commented 3 years ago

Apparently it's now complaining about databaseId, fixed in PR #51

PRijnbeek commented 3 years ago

We can setup a call to look into this. Please share the error message.

sdebruyn commented 3 years ago

the error message is the same, but now it's complaining about databaseId

The reason is that you run the code as in CodeToRun.R, but an R function should not be dependent on global scoped variables. In the PR I added the missing variables as arguments of the function, which resolves the issue.

PRijnbeek commented 3 years ago

Yes of course I agree @sdebruyn I know this :) It must be a left over not found earlier by others since they used the CodeToRun.

Thanks for mentioning this we will fix this. I assume you then now have a report and we look forward to looking into the results of your mapping in our next call. I will close this issue now and will deal with the pull request later today.