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

New feature: System diagnostics functions #98

Open
wants to merge 26 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
195c980
Created initial diagnostics script with proxy checking function
rmbielby Nov 8, 2024
60a349e
Added RENV_DOWNLOAD_METHOD diagnostic script
rmbielby Nov 8, 2024
f02301f
Simplified diagnostic test workflows
rmbielby Nov 11, 2024
3d23dfd
Lintr and code styling
rmbielby Nov 11, 2024
e18e029
More lintr and code styling
rmbielby Nov 11, 2024
faa6a6e
Added some description to diagnostic scripts
rmbielby Nov 11, 2024
f029e76
Added a test for the renv download method diagnostic check
rmbielby Nov 11, 2024
89a7d4c
Updating diagnostics documentation
rmbielby Nov 11, 2024
42e0600
Added GITHUB_PAT check to diagnostic tools
rmbielby Nov 12, 2024
03f0437
Adapted proxy settings check output to be more consistent with rest o…
rmbielby Nov 12, 2024
0d1e2f2
Cleaning up and updating pkgdown with check github_pat
rmbielby Nov 12, 2024
1853b75
Accounting for GitHub Actions GITHUB_PAT system variable of ***
rmbielby Nov 12, 2024
17ed957
Switching messages on for GITHUB_PAT check to help diagnose issue on …
rmbielby Nov 12, 2024
f1c7bee
Switching messages on for GITHUB_PAT check to help diagnose issue on …
rmbielby Nov 12, 2024
545fe68
Switching messages on for GITHUB_PAT check to help diagnose issue on …
rmbielby Nov 12, 2024
1970f0f
Switching messages on for GITHUB_PAT check to help diagnose issue on …
rmbielby Nov 12, 2024
4fb6e61
Switching messages on for GITHUB_PAT check to help diagnose issue on …
rmbielby Nov 12, 2024
fd8d896
Switching messages on for GITHUB_PAT check to help diagnose issue on …
rmbielby Nov 12, 2024
1ea5261
Removing GITHUB_PAT check test as GitHub actions won't return the env…
rmbielby Nov 12, 2024
ec631aa
Code styling
rmbielby Nov 12, 2024
7b9095d
Expanded proxy settings check to include environment variables
rmbielby Jan 3, 2025
ebcb2d6
Few bits of small clean-up on the proxy diagnostics
rmbielby Jan 3, 2025
840a9f3
Added sslverify check
rmbielby Jan 3, 2025
ea3f9c2
Merge branch 'main' into feature/sys-diagnostics
rmbielby Jan 3, 2025
0f1557c
Added beginnings of function to check location of the Git global conf…
rmbielby Jan 3, 2025
f9341d2
Merge branch 'feature/sys-diagnostics' of https://github.com/dfe-anal…
rmbielby Jan 3, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions DESCRIPTION
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ Depends:
Imports:
dplyr,
emoji,
git2r,
httr,
jsonlite,
lifecycle,
Expand Down
6 changes: 6 additions & 0 deletions NAMESPACE
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
# Generated by roxygen2: do not edit by hand

export(check_git_sslverify)
export(check_gitconfig_location)
export(check_github_pat)
export(check_proxy_settings)
export(check_renv_download_method)
export(comma_sep)
export(create_project)
export(diagnostic_test)
export(fetch_countries)
export(fetch_lads)
export(fetch_las)
Expand Down
324 changes: 324 additions & 0 deletions R/diagnostic_test.R
Original file line number Diff line number Diff line change
@@ -0,0 +1,324 @@
#' Diagnostic testing
#'
#' @description
#' Run a set of diagnostic tests to check for common issues found when setting
#' up R on a DfE
#' system. This includes:
#' - Checking for proxy settings in the Git configuration
#' - Checking for correct download method used by renv (curl)
#'
#' @inheritParams check_proxy_settings
#'
#' @return List object of detected parameters
#' @export
#'
#' @examples
#' diagnostic_test()
diagnostic_test <- function(
clean = FALSE,
verbose = FALSE) {
results <- c(
check_proxy_settings(clean = clean, verbose = verbose),
check_git_sslverify(clean = clean, verbose = verbose),
check_github_pat(clean = clean, verbose = verbose),
check_renv_download_method(clean = clean, verbose = verbose)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we add an additional check for http.sslVerify in their Git config too?

system("git config --global http.sslVerify false") - was a line in the original proxy script and we should be discouraging analysts from having it in their configs anymore (as it's not longer necessary)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a check to switch it to TRUE instead if it finds it's set to false.

)
return(results)
}

#' Title
#'
#' @param clean
#'
#' @returns MULL
#' @export
#'
#' @examples
#' check_gitconfig_location()
check_gitconfig_location <- function(
clean = FALSE) {
config_files <- git2r:::git_config_files()
global_path <- config_files |>
dplyr::filter(file == "global") |>
dplyr::pull("path")
if (grepl("Users", global_path)) {
message(
"FAIL: Your global .gitconfig file is saved in a non-standard location."
)
message("It may not be recognised and read in when using Git.")
message("We recommend using the standard location:")
message(" C:/Users/<username>/.gitconfig")
message("It contains the following settings:")
print(git2r::config() |> magrittr::extract2("global"))
}
}


#' Check proxy settings
#'
#' @description
#' This script checks for "bad" proxy settings. Prior to the pandemic, analysts
#' in the DfE would need to add some proxy settings to either their GitHub
#' config or environment variables.
#' These settings now prevent Git from connecting to remote archives on GitHub
#' and Azure DevOps, so this script identifies and (if clean=TRUE is set)
#' removes them.
#'
#' @param proxy_setting_names Vector of proxy parameters to check for. Default:
#' c("http.proxy", "https.proxy")
#' @param clean Attempt to clean settings
#' @param verbose Run in verbose mode
#'
#' @return List of problem proxy settings
#' @export
#'
#' @examples
#' check_proxy_settings()
check_proxy_settings <- function(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't seem to be working, I've set http.proxy in my Git config
image

But running the function even after restarting R Studio doesn't seem to pick it up? Am I doing something wrong here?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you set that as a local repo level param or global? If it's global the code should pick it up, but if it's just local to the repo it won't.

proxy_setting_names = c("http.proxy", "https.proxy"),
clean = FALSE,
verbose = FALSE) {
proxy_settings_detected <- FALSE
# Check for proxy settings in the Git configuration
git_config <- git2r::config()
proxy_config <- git_config |>
magrittr::extract2("global") |>
magrittr::extract(proxy_setting_names)
proxy_config <- proxy_config[!is.na(names(proxy_config))]
if (any(!is.na(names(proxy_config)))) {
dfeR::toggle_message(
"Found proxy settings in Git config:",
verbose = verbose
)
paste(names(proxy_config), "=", proxy_config, collapse = "\n") |>
toggle_message(verbose = verbose)
proxy_settings_detected <- TRUE
if (clean) {
proxy_args <- proxy_config |>
lapply(function(list) {
NULL
})
rlang::inject(git2r::config(!!!proxy_args, global = TRUE))
message("FIXED: Git proxy settings have been cleared.")
} else {
message("FAIL: Git proxy setting have been left in place.")
}
} else {
proxy_config <- NULL
}
# Check for proxy settings set in the system environment (dfeR version)
# The sys variables have a slightly different syntax with "_" rather than "."
proxy_sysvar_names <- gsub("\\.", "_", proxy_setting_names)
proxy_system <- Sys.getenv(proxy_sysvar_names) |>
as.list()
proxy_system <- proxy_system[proxy_system != ""]
if (any(proxy_system != "")) {
dfeR::toggle_message(
"Found proxy settings in system environment:",
verbose = verbose
)
paste(names(proxy_system), "=", proxy_system, collapse = "\n") |>
toggle_message(verbose = verbose)
proxy_settings_detected <- TRUE
if (clean) {
proxy_args <- proxy_system |>
lapply(function(list) {
""
})
rlang::inject(Sys.setenv(!!!proxy_args))
message("FIXED: System environment proxy settings have been cleared.")
} else {
message(
"FAIL: System environment proxy settings have been left in place."
)
}
} else {
proxy_system <- NULL
}
if (!proxy_settings_detected) {
message(
"PASS: No proxy settings found in your Git config or system environment."
)
}
return(list(git = proxy_config, system = proxy_system))
}

#' Check Git sslverify setting
#'
#' @description
#' This script checks the values of the specified settings in the Git config and
#' if they're set to FALSE, it changes them to "TRUE" if clean=TRUE.
#' The defaut is to check for "http.sslVerify" and "https.sslVerify"
#'
#' @param ssl_verify_vars Vector of variables to check for in the Git config
#' @param clean Attempt to clean settings
#' @param verbose Run in verbose mode
#'
#' @return List of sslverify settings
#' @export
#'
#' @examples
#' check_git_sslverify()
check_git_sslverify <- function(
ssl_verify_vars = c("http.sslverify", "https.sslverify"),
clean = FALSE,
verbose = FALSE) {
# Check for proxy settings in the Git configuration
git_config <- git2r::config() |>
magrittr::extract2("global") |>
magrittr::extract(ssl_verify_vars)
git_config <- git_config[!is.na(names(git_config))]
if (any(!is.na(names(git_config)))) {
dfeR::toggle_message(
"Found specified settings in Git config:",
verbose = verbose
)
paste(names(git_config), "=", git_config, collapse = "\n") |>
toggle_message(verbose = verbose)
if (any(tolower(git_config) == "false")) {
if (clean) {
git_args <- git_config[tolower(git_config) == "false"] |>
lapply(function(list) {
"TRUE"
})
rlang::inject(git2r::config(!!!git_args, global = TRUE))
message("FIXED: Specified Git settings have been set")
} else {
message("FAIL: sslverify is set to FALSE.")
message("Specified Git settings have been left in place.")
}
} else {
message(
"PASS: sslverify is set to TRUE."
)
}
} else {
git_config <- NULL
message(
"PASS: sslverify is not explicitly set."
)
}
return(list(ssl_verify = git_config))
}


#' Check GITHUB_PAT setting
#'
#' @description
#' If the GITHUB_PAT keyword is set, then it can cause issues with R installing
#' packages from GitHub (usually with an error of "ERROR \[curl: (22) The
#' requested URL returned error: 401\]"). This script checks whether the keyword
#' is set and can then clear it (if clear=TRUE).
#' The user will then need to identify where the "GITHUB_PAT" variable is being
#' set from and remove it to permanently fix the issue.
#'
#' @inheritParams check_proxy_settings
#'
#' @return List object containing the github_pat keyword content.
#' @export
#'
#' @examples
#' check_github_pat()
check_github_pat <- function(
clean = FALSE,
verbose = FALSE) {
github_pat <- Sys.getenv("GITHUB_PAT")
# Replace above to remove non alphanumeric characters when run on GitHub
# Actions
if (github_pat != "") {
message(
"FAIL: GITHUB_PAT is set to ",
github_pat,
". This may cause issues with installing packages from GitHub",
" such as dfeR and dfeshiny."
)
if (clean) {
message("Clearing GITHUB_PAT keyword from system settings.")
Sys.unsetenv("GITHUB_PAT")
message(
"This issue may recur if you have some software that is",
"initialising the GITHUB_PAT keyword automatically."
)
}
} else {
message("PASS: The GITHUB_PAT system variable is clear.")
}
return(list(GITHUB_PAT = github_pat))
}

#' Check renv download method
#'
#' @description
#' The renv package can retrieve packages either using curl or wininet, but
#' wininet doesn't work from within the DfE network. This script checks for
#' the parameter controlling which of these is used (RENV_DOWNLOAD_METHOD) and
#' sets it to use curl.
#'
#' @param renviron_file Location of .Renviron file. Default: ~/.Renviron
#' @inheritParams check_proxy_settings
#'
#' @return List object containing RENV_DOWNLOAD_METHOD
#' @export
#'
#' @examples
#' check_renv_download_method()
check_renv_download_method <- function(
renviron_file = "~/.Renviron",
clean = FALSE,
verbose = FALSE) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel like the verbose = FALSE option should only print the success message, feels a bit too verbose printing the variable as well as the message?

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, this is just that it's returning something and it's printing that out. I could switch the behaviour to not return anything, but given it's a diagnostic tool, I figure the individual functions should return some sort of output to feed in to the master diagnostic function for info.

if (file.exists(renviron_file)) {
.renviron <- readLines(renviron_file)
} else {
.renviron <- c()
}
rdm_present <- .renviron %>% stringr::str_detect("RENV_DOWNLOAD_METHOD")
if (any(rdm_present)) {
current_setting_message <- paste0(
"RENV_DOWNLOAD_METHOD is currently set to:\n ",
.renviron[rdm_present]
)
detected_method <- .renviron[rdm_present] |>
stringr::str_split("=") |>
unlist() |>
magrittr::extract(2)
} else {
current_setting_message <- "RENV_DOWNLOAD_METHOD is not currently set."
detected_method <- NA
}
if (is.na(detected_method) || detected_method != "\"curl\"") {
if (clean) {
if (any(rdm_present)) {
.renviron <- .renviron[!rdm_present]
}
.renviron <- c(
.renviron,
"RENV_DOWNLOAD_METHOD=\"curl\""
)
cat(.renviron, file = renviron_file, sep = "\n")
message(
paste0(
"FIXED: The renv download method has been set to curl in your ",
".Renviron file."
)
)
readRenviron(renviron_file)
} else {
dfeR::toggle_message(paste("FAIL:", current_setting_message),
verbose = verbose
)
message("If you wish to manually update your .Renviron file:")
message(" - Run the command in the R console to open .Renviron:")
message(" usethis::edit_r_environ()")
if (any(rdm_present)) {
message(" - Remove the following line from .Renviron:")
message(" ", .renviron[rdm_present])
}
message(" - Add the following line to .Renviron:")
message(" RENV_DOWNLOAD_METHOD=\"curl\"")
message("Or run `dfeR::check_renv_download_method(clean=TRUE)`")
}
} else {
message("PASS: Your RENV_DOWNLOAD_METHOD is set to curl.")
}
return(list(RENV_DOWNLOAD_METHOD = detected_method))
}
8 changes: 8 additions & 0 deletions _pkgdown.yml
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,14 @@ reference:
contents:
- starts_with("format_")

- title: System diagnostics
desc: Functions to diagnose and fix common system issues
contents:
- diagnostic_test
- check_proxy_settings
- check_github_pat
- check_renv_download_method

- title: Helpers
desc: >
Functions used in other functions across the package, exported as they can also
Expand Down
1 change: 1 addition & 0 deletions dfeR.Rproj
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
Version: 1.0
ProjectId: 27106e85-ccc2-4843-b5cd-3833d61941bb

RestoreWorkspace: No
SaveWorkspace: No
Expand Down
30 changes: 30 additions & 0 deletions man/check_git_sslverify.Rd

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading