From 125b8aeb92d840c8ddc2b5d5c072e17622d31b71 Mon Sep 17 00:00:00 2001 From: Anatoliy Sokolov Date: Wed, 30 Oct 2024 14:33:39 -0400 Subject: [PATCH 1/5] updates token handling for Azure --- NAMESPACE | 1 + R/cache.R | 7 +++++ R/service-azure_openai.R | 44 +++++++++++++++++--------------- R/zzz.R | 3 +++ man/gptstudio_cache_directory.Rd | 11 ++++++++ 5 files changed, 46 insertions(+), 20 deletions(-) create mode 100644 R/cache.R create mode 100644 man/gptstudio_cache_directory.Rd diff --git a/NAMESPACE b/NAMESPACE index 46fd2c12..9eaaa51c 100644 --- a/NAMESPACE +++ b/NAMESPACE @@ -41,6 +41,7 @@ export(create_completion_huggingface) export(get_available_endpoints) export(get_available_models) export(get_ide_theme_info) +export(gptstudio_cache_directory) export(gptstudio_chat) export(gptstudio_chat_in_source_addin) export(gptstudio_comment_code) diff --git a/R/cache.R b/R/cache.R new file mode 100644 index 00000000..22828c53 --- /dev/null +++ b/R/cache.R @@ -0,0 +1,7 @@ +#' a function that determines the appropriate directory to cache a token +#' @export +gptstudio_cache_directory = function(){ + rappdirs::user_data_dir(appname = glue::glue("gptstudio"), + appauthor = "", + roaming = FALSE) +} diff --git a/R/service-azure_openai.R b/R/service-azure_openai.R index a24dce46..0d4f1412 100644 --- a/R/service-azure_openai.R +++ b/R/service-azure_openai.R @@ -110,31 +110,33 @@ query_api_azure_openai <- retrieve_azure_token <- function() { rlang::check_installed("AzureRMR") - token <- tryCatch( - { - AzureRMR::get_azure_login( - tenant = Sys.getenv("AZURE_OPENAI_TENANT_ID"), - app = Sys.getenv("AZURE_OPENAI_CLIENT_ID"), - scopes = ".default" - ) - }, - error = function(e) NULL - ) + token <- retrieve_azure_token_object() %>% suppressMessages() - if (is.null(token)) { - token <- AzureRMR::create_azure_login( - tenant = Sys.getenv("AZURE_OPENAI_TENANT_ID"), - app = Sys.getenv("AZURE_OPENAI_CLIENT_ID"), - password = Sys.getenv("AZURE_OPENAI_CLIENT_SECRET"), - host = "https://cognitiveservices.azure.com/", - scopes = ".default" - ) - } + invisible(token$credentials$access_token) +} + + +retrieve_azure_token_object <- function() { + ## Set this so that do_login properly caches + azure_data_env = Sys.getenv("R_AZURE_DATA_DIR") + Sys.setenv("R_AZURE_DATA_DIR" = gptstudio_cache_directory()) - invisible(token$token$credentials$access_token) + client <- Microsoft365R:::do_login(tenant = Sys.getenv("AZURE_OPENAI_TENANT_ID"), + app = Sys.getenv("AZURE_OPENAI_CLIENT_ID"), + host = Sys.getenv("AZURE_OPENAI_SCOPE"), + scopes = NULL, + auth_type = "client_credentials", + password = Sys.getenv("AZURE_OPENAI_CLIENT_SECRET"), + token = NULL) + ## Set this so that do_login properly caches + Sys.setenv("R_AZURE_DATA_DIR" = azure_data_env) + + invisible(client$token) } + + stream_azure_openai <- function(messages = list(list(role = "user", content = "hi there")), element_callback = cat) { body <- list( @@ -158,3 +160,5 @@ stream_azure_openai <- function(messages = list(list(role = "user", content = "h invisible(response) } + + diff --git a/R/zzz.R b/R/zzz.R index e2535db7..57195d6a 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -10,6 +10,9 @@ )) } + require("Microsoft365R") + if(!dir.exists(gptstudio_cache_directory())) dir.create(gptstudio_cache_directory()) + op <- options() op_gptstudio <- list( diff --git a/man/gptstudio_cache_directory.Rd b/man/gptstudio_cache_directory.Rd new file mode 100644 index 00000000..6d8b4ccf --- /dev/null +++ b/man/gptstudio_cache_directory.Rd @@ -0,0 +1,11 @@ +% Generated by roxygen2: do not edit by hand +% Please edit documentation in R/cache.R +\name{gptstudio_cache_directory} +\alias{gptstudio_cache_directory} +\title{a function that determines the appropriate directory to cache a token} +\usage{ +gptstudio_cache_directory() +} +\description{ +a function that determines the appropriate directory to cache a token +} From 829aadef3d907461b79a628b8358249340b781f0 Mon Sep 17 00:00:00 2001 From: Anatoliy Sokolov Date: Thu, 5 Dec 2024 15:40:16 -0500 Subject: [PATCH 2/5] Swapping Microsoft365R for AzureGraph, small bug fixes to streamline token handling. --- DESCRIPTION | 2 +- R/cache.R | 4 +--- R/service-azure_openai.R | 35 +++++++++++++++++++++++------------ R/zzz.R | 1 - 4 files changed, 25 insertions(+), 17 deletions(-) diff --git a/DESCRIPTION b/DESCRIPTION index 10a6d179..04dcc6fe 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -42,7 +42,7 @@ Imports: utils, yaml Suggests: - AzureRMR, + AzureGraph, future, grDevices, knitr, diff --git a/R/cache.R b/R/cache.R index 22828c53..5989c64f 100644 --- a/R/cache.R +++ b/R/cache.R @@ -1,7 +1,5 @@ #' a function that determines the appropriate directory to cache a token #' @export gptstudio_cache_directory = function(){ - rappdirs::user_data_dir(appname = glue::glue("gptstudio"), - appauthor = "", - roaming = FALSE) + tools::R_user_dir(package = "gptstudio") } diff --git a/R/service-azure_openai.R b/R/service-azure_openai.R index 0d4f1412..6eb33f7a 100644 --- a/R/service-azure_openai.R +++ b/R/service-azure_openai.R @@ -108,30 +108,41 @@ query_api_azure_openai <- } retrieve_azure_token <- function() { - rlang::check_installed("AzureRMR") - token <- retrieve_azure_token_object() %>% suppressMessages() + token <- retrieve_azure_token_object() |> suppressMessages() invisible(token$credentials$access_token) } retrieve_azure_token_object <- function() { - ## Set this so that do_login properly caches + rlang::check_installed("AzureGraph") + + ## Set this so that get_graph_login properly caches azure_data_env = Sys.getenv("R_AZURE_DATA_DIR") Sys.setenv("R_AZURE_DATA_DIR" = gptstudio_cache_directory()) - client <- Microsoft365R:::do_login(tenant = Sys.getenv("AZURE_OPENAI_TENANT_ID"), - app = Sys.getenv("AZURE_OPENAI_CLIENT_ID"), - host = Sys.getenv("AZURE_OPENAI_SCOPE"), - scopes = NULL, - auth_type = "client_credentials", - password = Sys.getenv("AZURE_OPENAI_CLIENT_SECRET"), - token = NULL) - ## Set this so that do_login properly caches + login <- try(AzureGraph::get_graph_login(tenant = Sys.getenv("AZURE_OPENAI_TENANT_ID"), + app = Sys.getenv("AZURE_OPENAI_CLIENT_ID"), + scopes = NULL, + refresh = FALSE), + silent=TRUE) |> + suppressMessages() + + if(inherits(login, "try-error")) { + login <- AzureGraph::create_graph_login(tenant = Sys.getenv("AZURE_OPENAI_TENANT_ID"), + app = Sys.getenv("AZURE_OPENAI_CLIENT_ID"), + host = Sys.getenv("AZURE_OPENAI_SCOPE"), + scopes = NULL, + auth_type = "client_credentials", + password = Sys.getenv("AZURE_OPENAI_CLIENT_SECRET")) |> + suppressMessages() + } + + ## Set this so that get_graph_login properly caches Sys.setenv("R_AZURE_DATA_DIR" = azure_data_env) - invisible(client$token) + invisible(login$token) } diff --git a/R/zzz.R b/R/zzz.R index 57195d6a..6fa1ce47 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -10,7 +10,6 @@ )) } - require("Microsoft365R") if(!dir.exists(gptstudio_cache_directory())) dir.create(gptstudio_cache_directory()) op <- options() From b35ebb3f91bcdd29132a66f347adcf5d987dced1 Mon Sep 17 00:00:00 2001 From: Anatoliy Sokolov Date: Thu, 5 Dec 2024 16:47:40 -0500 Subject: [PATCH 3/5] Adjusting tests for the new token handling functions --- tests/testthat/test-service-azure_openai.R | 49 ++++++++++++---------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/tests/testthat/test-service-azure_openai.R b/tests/testthat/test-service-azure_openai.R index c78192f9..d9cbbb4a 100644 --- a/tests/testthat/test-service-azure_openai.R +++ b/tests/testthat/test-service-azure_openai.R @@ -154,11 +154,11 @@ test_that("query_api_azure_openai handles error response", { test_that("retrieve_azure_token successfully gets existing token", { local_mocked_bindings( - get_azure_login = function(...) { - list(token = list(credentials = list(access_token = "existing_token"))) + get_graph_login = function(...) { + list(credentials = list(access_token = "existing_token")) }, - create_azure_login = function(...) stop("Should not be called"), - .package = "AzureRMR" + create_graph_login = function(...) stop("Should not be called"), + .package = "AzureGraph" ) token <- retrieve_azure_token() @@ -166,13 +166,13 @@ test_that("retrieve_azure_token successfully gets existing token", { expect_equal(token, "existing_token") }) -test_that("retrieve_azure_token creates new token when get_azure_login fails", { +test_that("retrieve_azure_token creates new token when get_graph_login fails", { local_mocked_bindings( - get_azure_login = function(...) stop("Error"), - create_azure_login = function(...) { - list(token = list(credentials = list(access_token = "new_token"))) + get_graph_login = function(...) stop("Error"), + create_graph_login = function(...) { + list(credentials = list(access_token = "new_token")) }, - .package = "AzureRMR" + .package = "AzureGraph" ) token <- retrieve_azure_token() @@ -180,41 +180,48 @@ test_that("retrieve_azure_token creates new token when get_azure_login fails", { expect_equal(token, "new_token") }) + test_that("retrieve_azure_token uses correct environment variables", { - mock_get_azure_login <- function(tenant, app, scopes) { + mock_get_graph_login <- function(tenant, app, scopes, refresh) { expect_equal(tenant, "test_tenant") expect_equal(app, "test_client") - expect_equal(scopes, ".default") + expect_equal(scopes, NULL) + expect_equal(refresh, FALSE) stop("Error") } - mock_create_azure_login <- function(tenant, app, password, host, scopes) { + mock_create_graph_login <- function(tenant, app, host, scopes, auth_type, password) { expect_equal(tenant, "test_tenant") expect_equal(app, "test_client") + expect_equal(host, "https://cognitiveservices.azure.com/.default") + expect_equal(scopes, NULL) + expect_equal(auth_type, "client_credentials") expect_equal(password, "test_secret") - expect_equal(host, "https://cognitiveservices.azure.com/") - expect_equal(scopes, ".default") - list(token = list(credentials = list(access_token = "new_token"))) + list(credentials = list(access_token = "new_token")) } local_mocked_bindings( - get_azure_login = mock_get_azure_login, - create_azure_login = mock_create_azure_login, - .package = "AzureRMR" + get_graph_login = mock_get_graph_login, + create_graph_login = mock_create_graph_login, + .package = "AzureGraph" ) withr::local_envvar( AZURE_OPENAI_TENANT_ID = "test_tenant", AZURE_OPENAI_CLIENT_ID = "test_client", - AZURE_OPENAI_CLIENT_SECRET = "test_secret" + AZURE_OPENAI_CLIENT_SECRET = "test_secret", + AZURE_OPENAI_SCOPE = "https://cognitiveservices.azure.com/.default" ) expect_no_error(retrieve_azure_token()) }) -test_that("retrieve_azure_token checks for AzureRMR installation", { + + + +test_that("retrieve_azure_token checks for AzureGraph installation", { mock_check_installed <- function(pkg) { - expect_equal(pkg, "AzureRMR") + expect_equal(pkg, "AzureGraph") } local_mocked_bindings( From 5624b4f8f724b344215a33925ec870ce1c25521d Mon Sep 17 00:00:00 2001 From: Anatoliy Sokolov Date: Tue, 17 Dec 2024 11:07:16 -0500 Subject: [PATCH 4/5] skip tests on CI and dont create token cache directory until needed. --- R/service-azure_openai.R | 5 +++++ R/zzz.R | 2 -- tests/testthat/test-service-azure_openai.R | 6 ++++++ 3 files changed, 11 insertions(+), 2 deletions(-) diff --git a/R/service-azure_openai.R b/R/service-azure_openai.R index 6eb33f7a..7c73ae51 100644 --- a/R/service-azure_openai.R +++ b/R/service-azure_openai.R @@ -120,6 +120,7 @@ retrieve_azure_token_object <- function() { ## Set this so that get_graph_login properly caches azure_data_env = Sys.getenv("R_AZURE_DATA_DIR") + Sys.setenv("R_AZURE_DATA_DIR" = gptstudio_cache_directory()) login <- try(AzureGraph::get_graph_login(tenant = Sys.getenv("AZURE_OPENAI_TENANT_ID"), @@ -130,6 +131,10 @@ retrieve_azure_token_object <- function() { suppressMessages() if(inherits(login, "try-error")) { + + if(!dir.exists(gptstudio_cache_directory())) dir.create(gptstudio_cache_directory()) |> suppressWarnings() + + login <- AzureGraph::create_graph_login(tenant = Sys.getenv("AZURE_OPENAI_TENANT_ID"), app = Sys.getenv("AZURE_OPENAI_CLIENT_ID"), host = Sys.getenv("AZURE_OPENAI_SCOPE"), diff --git a/R/zzz.R b/R/zzz.R index 6fa1ce47..e2535db7 100644 --- a/R/zzz.R +++ b/R/zzz.R @@ -10,8 +10,6 @@ )) } - if(!dir.exists(gptstudio_cache_directory())) dir.create(gptstudio_cache_directory()) - op <- options() op_gptstudio <- list( diff --git a/tests/testthat/test-service-azure_openai.R b/tests/testthat/test-service-azure_openai.R index d9cbbb4a..cd5dd038 100644 --- a/tests/testthat/test-service-azure_openai.R +++ b/tests/testthat/test-service-azure_openai.R @@ -153,6 +153,8 @@ test_that("query_api_azure_openai handles error response", { # Test token retrieval -------------------------------------------------------- test_that("retrieve_azure_token successfully gets existing token", { + skip_on_ci() + local_mocked_bindings( get_graph_login = function(...) { list(credentials = list(access_token = "existing_token")) @@ -167,6 +169,8 @@ test_that("retrieve_azure_token successfully gets existing token", { }) test_that("retrieve_azure_token creates new token when get_graph_login fails", { + skip_on_ci() + local_mocked_bindings( get_graph_login = function(...) stop("Error"), create_graph_login = function(...) { @@ -182,6 +186,8 @@ test_that("retrieve_azure_token creates new token when get_graph_login fails", { test_that("retrieve_azure_token uses correct environment variables", { + skip_on_ci() + mock_get_graph_login <- function(tenant, app, scopes, refresh) { expect_equal(tenant, "test_tenant") expect_equal(app, "test_client") From 6488ac824c685922341b4446fea88d05930ea54a Mon Sep 17 00:00:00 2001 From: Anatoliy Sokolov Date: Tue, 17 Dec 2024 11:45:43 -0500 Subject: [PATCH 5/5] linting --- R/cache.R | 2 +- R/service-azure_openai.R | 14 +++++++------- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/R/cache.R b/R/cache.R index 5989c64f..8d367584 100644 --- a/R/cache.R +++ b/R/cache.R @@ -1,5 +1,5 @@ #' a function that determines the appropriate directory to cache a token #' @export -gptstudio_cache_directory = function(){ +gptstudio_cache_directory <- function() { tools::R_user_dir(package = "gptstudio") } diff --git a/R/service-azure_openai.R b/R/service-azure_openai.R index 7c73ae51..035d9258 100644 --- a/R/service-azure_openai.R +++ b/R/service-azure_openai.R @@ -119,7 +119,7 @@ retrieve_azure_token_object <- function() { rlang::check_installed("AzureGraph") ## Set this so that get_graph_login properly caches - azure_data_env = Sys.getenv("R_AZURE_DATA_DIR") + azure_data_env <- Sys.getenv("R_AZURE_DATA_DIR") Sys.setenv("R_AZURE_DATA_DIR" = gptstudio_cache_directory()) @@ -127,12 +127,15 @@ retrieve_azure_token_object <- function() { app = Sys.getenv("AZURE_OPENAI_CLIENT_ID"), scopes = NULL, refresh = FALSE), - silent=TRUE) |> + silent = TRUE) |> suppressMessages() - if(inherits(login, "try-error")) { + if (inherits(login, "try-error")) { - if(!dir.exists(gptstudio_cache_directory())) dir.create(gptstudio_cache_directory()) |> suppressWarnings() + if (!dir.exists(gptstudio_cache_directory())) { + dir.create(gptstudio_cache_directory()) |> + suppressWarnings() + } login <- AzureGraph::create_graph_login(tenant = Sys.getenv("AZURE_OPENAI_TENANT_ID"), @@ -173,8 +176,5 @@ stream_azure_openai <- function(messages = list(list(role = "user", content = "h }, round = "line" ) - invisible(response) } - -