Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

golem::document_and_reload is not using get_golem_config to figure out where the golem configuration is #887

Open
harell opened this issue Jul 22, 2022 · 6 comments

Comments

@harell
Copy link

harell commented Jul 22, 2022

Hi

Thanks for the great work.

Given I have moved the default "./inst/golem-config.yml" to "./inst/config/golem.yml"
AND declare the configuration file path in "/R/get_golem_config.R"
When I run "./dev/run_dev.R"
Then script fails to find the config file in the updated address

Failure Message

image

get_golem_config

get_golem_config <- function(
  value,
  config = Sys.getenv("GOLEM_CONFIG_ACTIVE", Sys.getenv("R_CONFIG_ACTIVE","default")),
  use_parent = TRUE,
  # Modify this if your config file is somewhere else
  file = app_sys("config/golem.yml")
) {
  config::get(
    value = value,
    config = config,
    file = file,
    use_parent = use_parent
  )
}

Note I updated file = app_sys("config/golem.yml") from its original file = app_sys("golem-config.yml")

Failure Traceback

image

  • None of the functions above uses get_golem_config to retrieve information from the configuration file.

Please advise how to change the location of the yml file and run /run_dev.R successfully.

@ALanguillaume
Copy link
Member

ALanguillaume commented Jul 22, 2022

Hi @harell,

Thanks for reporting this.

TLDR: Currently you cannot change the location of inst/golem-config.yml despite what the comment in
get_golem_config() says. Apologies for that. We will try to make it happen or at least less confusing in the future versions of golem.

Indeed, despite the comment in the code of get_golem_config() (R/app_config.R)

# Modify this if your config file is somewhere else
  file = app_sys("config/golem-config.yml")

It is not currently possible to change the location of the golem-config.yml.

Doing so will break a number of golem functions that rely on golem-config.yml to get information about the current golem project.
For example in your case the problem arises when document_and_reload() tries to determine the current working directory.

document_and_reload()
  |-> get_golem_wd()
    |-> get_golem_things()
      |-> get_current_config()

In current get_current_config() the path to inst/golem-config.yml is hardcoded.

Here is a reprex of that issue:

# Using golem last dev version
packageVersion("golem")
#> [1] '0.3.2.9004'

path_dummy_golem <- tempfile(pattern = "dummygolem")

golem::create_golem(
  path = path_dummy_golem,
  open = FALSE
)
#> ── Checking package name ───────────────────────────────────────────────────────
#> ✔ Valid package name
#> ── Creating dir ────────────────────────────────────────────────────────────────
#> ✔ Creating '/tmp/RtmpFJejPj/dummygolem74271f138c71/'
#> ✔ Setting active project to '/tmp/RtmpFJejPj/dummygolem74271f138c71'
#> ✔ Creating 'R/'
#> ✔ Writing a sentinel file '.here'
#> • Build robust paths within your project via `here::here()`
#> • Learn more at <https://here.r-lib.org/>
#> ✔ Setting active project to '<no active project>'
#> ✔ Created package directory
#> ── Copying package skeleton ────────────────────────────────────────────────────
#> ✔ Copied app skeleton
#> ── Setting the default config ──────────────────────────────────────────────────
#> ✔ Configured app
#> ── Running project hook function ───────────────────────────────────────────────
#> ✔ All set
#> ✔ Setting active project to '/tmp/RtmpFJejPj/dummygolem74271f138c71'
#> ── Done ────────────────────────────────────────────────────────────────────────
#> A new golem named dummygolem74271f138c71 was created at /tmp/RtmpFJejPj/dummygolem74271f138c71 .
#> To continue working on your app, start editing the 01_start.R file.

old_wd <- setwd(path_dummy_golem)

# Works fine
golem::document_and_reload()
#> Setting `RoxygenNote` to "7.2.0"
#> ℹ Loading dummygolem74271f138c71
#> Writing 'run_app.Rd'
#> ℹ Loading dummygolem74271f138c71

# Move config file
dir.create(
  "inst/config"
)
file.copy(
  from = "inst/golem-config.yml",
  to = "inst/config/golem-config.yml"
)
#> [1] TRUE
file.remove(
  "inst/golem-config.yml"
)
#> [1] TRUE

# {golem} cannot find golem-config.yml
golem::document_and_reload()
#> Error in get_current_config(path, set_options = TRUE): The golem-config.yml file doesn't exist.

# Edit R/app_config.R won't help
path_app_config <- "R/app_config.R"
writeLines(
  sub(
    pattern = "golem-config\\.yml", 
    replacement = "config/golem-config.yml",
    readLines(path_app_config)
  ),
  path_app_config
)
pkgload::load_all()
#> ℹ Loading dummygolem74271f138c71
golem::document_and_reload()
#> Error in get_current_config(path, set_options = TRUE): The golem-config.yml file doesn't exist.

# The error is triggered by which remains agnostic
# of the change in location of golem-config.yml
golem::get_golem_wd()
#> Error in get_current_config(path, set_options = TRUE): The golem-config.yml file doesn't exist.

# This will also also append with other functions trying to 
# access golem-config.yml
golem::get_golem_name()
#> Error in get_current_config(path, set_options = TRUE): The golem-config.yml file doesn't exist.

## The main culprit is golem:::get_current_config() (internal function)
# Where the path to "golem-config.yml" is hardcoded to "inst/golem-config.yml"
golem:::get_current_config()
#> Error in golem:::get_current_config(): The golem-config.yml file doesn't exist.

@ALanguillaume
Copy link
Member

@ColinFay I started working on a fix in branch fix-change-config-yml-loc.
See thread on our Slack.

@ColinFay
Copy link
Member

@ALanguillaume are you still planning on working on this?

@ColinFay ColinFay added this to the Version 0.5.0 milestone Dec 15, 2022
@ilyaZar
Copy link
Contributor

ilyaZar commented May 24, 2023

Whenever golem::document_and_reload() calls get_golem_wd() to retrieve values from a yml-config, and the user changed the destination of that yml before that call, golem::document_and_reload() has no chance to know about that config change in the current setup.

So golem::document_and_reload() must be able to retrieve this information in case sensible default locations for the yaml-config fail to work via guess_where_config().

Currently, the user is asked to make changes to the location of the config-yaml in “R/app_config.R”
e.g. like taken from @Harrel’s initial bug report:

  # Modify this if your config file is somewhere else
  file = app_sys("config/golem.yml")

So that’s currently the best place for guess_where_config() to search.

Sorry for the prior mess in the now closed PR, but #1043 could be considered instead now.

@ColinFay
Copy link
Member

ColinFay commented Aug 7, 2024

Here is the current workaround :

  • in dev/run_dev.R : golem::document_and_reload(pkg = ".")

  • Then : golem::run_dev(pkg = ".")

Just so you know, the get_golem_wd() function relying on a config.yaml file is a CRAN requirement, bcs you can't manipulate the user dir without giving explicit notice & allowing to change it.

I'm currently torn about this, I'm not sure we would want to support changing where the config file is stored, given that, by definition, it's the config file and that moving it would require to hard write the location somewhere else 🤔

Just like you need the DESCRIPTION to be at the root of your package, we might want to make it a hard requirement. Plus, for example, how do we handle inst/config/golem-config.yml & inst/configuration/golem-config.yml ?

Another option would be to have an env var then defaulting to the current heuristic that looks for the file, but I feel like it adds a bit of complexity.

Happy to have your feedbacks about this.

@ilyaZar
Copy link
Contributor

ilyaZar commented Aug 7, 2024

Hi @ColinFay

I understand your point and having configs randomly flying in some folders is certainly unwise and not support good dev practices..

I have a feeling, though, that being able to have

  • inst/config/golem-config_01.yml
  • inst/config/golem-config_02.yml

gives extra freedom in switching configs quickly for dev/prod setups.

It's possible to hard-code the top-level path and having clear error messages when there is no config at all, or if the exact file name cannot be found?

Happy to make a change to my back then PR if you like

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants