openpharma / clinsight

ClinSight - An application for medical monitoring within clinical trials
https://openpharma.github.io/clinsight/
Other
3 stars 0 forks source link

Fully move `metadata` to application argument or configuration #18

Closed jthompson-arcus closed 4 months ago

jthompson-arcus commented 5 months ago

The metadata object bundled within the application will get called occasionally instead of the metadata object passed as an argument to the application at run time (e.g. R/app_server.R#L87, R/fct_SQLite.R#L79, etc.).

jthompson-arcus commented 5 months ago

If the metadata is moved to a configuration, then pre-processing and manual processing applications could still have access to the correct data without having to explicitly pass it.

LDSamson commented 5 months ago

@jthompson-arcus we probably should then move all data objects passed to the application to a config file and make calling these files in run_app optional, what do you think?

Secondly, should we use an environmental variable to provide the path to the config file to overwrite the one in the package if needed?

i.e. change this: https://github.com/openpharma/clinsight/blob/f29a720bd6a705241adf76843a4fc5195a51c7c0/R/app_config.R#L36

to something like

file = if(Sys.getenv("CONFIG_PATH") != "") Sys.getenv("CONFIG_PATH") else app_sys("golem-config.yml") 
jthompson-arcus commented 5 months ago

I think the path forward is through environmental variables. golem already has a somewhat baked in way to accomplish this with get_golem_config() in R/app_config.R. Depending on the value for "GOLEM_CONFIG_ACTIVE" or "R_CONFIG_ACTIVE", we can have different file paths for the metadata and also set the other arguments as well. The solution will need to work both when the application is being run and whatever manual processing is occurring outside of the app.

LDSamson commented 5 months ago

Sorry for not being clear. My point was: the file argument in the get_golem_config() function is fixed to the path to the golem-config.yml inside the package and thus you can only change the config file when you build the package from source again. For maximum flexibility, it is probably better to give users the option to provide their own config file. They will have to provide the path to this file in a separate environmental variable. You can find a (bit ugly) way to make this work in my previous post. If I am missing anything, please let me know.

I agree that if we only use the config file for filepaths, this addition is not essential, but for other configuration options such as "review by form", or to set the authority for resolving queries to custom user roles, this might be a nice addition.

jthompson-arcus commented 5 months ago

I see your point. My solution is too focused on git-backed deployments.

Shouldn't changes in metadata correspond with a redeployment of the application though? I would think changing the metadata would affect the SQLite database.

LDSamson commented 5 months ago

Do you usually have a git repo with a fork of the package for each study it will be deployed on?

Yes, the user_db.sqlite database will change dependent on the metadata. So you should only change configuration at the start of a study (when the database is created based on the provided metadata).