search

Appsilon / rhino

Build high quality, enterprise-grade Shiny apps at speed
https://appsilon.github.io/rhino
293 stars 25 forks source link

`R_CONFIG_ACTIVE` is a restricted environmental variable in RStudio Connect. #351

Open kamilzyla opened 2 years ago

kamilzyla commented 2 years ago

Background

In Posit Connect you can set environment variables for each application separately. You cannot set R_CONFIG_ACTIVE however - it is configured statically for the whole server using R.ConfigActive setting. By default this environment variable is used by config::get() to select the configuration to load.

Problem

Internally Rhino calls config::get() with default arguments (using R_CONFIG_ACTIVE) to read rhino_log_level and rhino_log_file settings from config.yml. If our users decide to use a different environment variable to control which config is loaded (e.g. they call config::get(config = Sys.getenv("TARGET_ENV", "default")) in their code), they will be unable to manage logging settings in the same way.

It is still possible to configure logging separately for different applications deployed to the same Posit Connect server (by default we read RHINO_LOG_LEVEL and RHINO_LOG_FILE environment variables) but it can make the configuration more complex and difficult to understand.

Notes

  1. Are we actually using config? (Appsilon-internal Slack discussion)
Leszek-Sieminski commented 2 years ago

Information from @jakubnowicki: R_CONFIG_ACTIVE is a default variable that controls the choice of configuration version (config package), but it is NOT allowed in RStudio Connect. This makes controlling different versions of logging impossible because Rhino uses the default variable

TymekDev commented 3 days ago

I think this one is a bug or borderline bug.

Updating config.yml to include:

prod:
  rhino_log_level: FATAL
  1. Won't be in effect outside of Connect. Unless the app uses R_CONFIG_ACTIVE to change the environments.
  2. Might or might not have effect on Connect. Depends on what the R_CONFIG_ACTIVE is set to.

I think 1. is has a low frequency - if someone can use the default R_CONFIG_ACTIVE, then chances are they will use it.

However, with 2., I think the frequency is much higher. Most of the apps I saw used a different environment variable to have the full control. This means that there will be a mismatch of variables - {rhino} uses R_CONFIG_ACTIVE and the app uses something else.

I didn't know about this until recently when I was browsing through {rhino}'s codebase.

I think the solution here is to configure the environment variable name via rhino.yml and pass it to config::get(): https://github.com/Appsilon/rhino/blob/7c6106f8857aa5ab647f22075b006f6191a34bb0/R/app.R#L75 It could fall back to R_CONFIG_ACTIVE if not set - this way it won't break existing applications.

Alternatives:

  1. Have the user explicitly set it. rhino::set_log_level()? The user is in charge.
  2. Resign completely from wrapping {logger}. Currently, rhino::log is quite a thin wrapper on logger. Perhaps encouraging users to use {logger} directly would make sense?