Proposal: Make `client` optional, with an implicit or explicitly-set default value.

[toph-allen]

Code in connectapi uses the Connect class to construct URLs and make requests to servers. The Connect object is currently made available to code in two ways:

1. Passed in explicitly as the first argument of many functions, such as content_item() and get_groups().
2. Provided implicitly to functions that operate other R6 classes, as those R6 classes (such as Content) contain a reference to the client object in their $connect property.

The proposal is to make client an optional argument, trailing in the arg list, with a configurable default value if none is provided. To use a non-default client, for instance with multi-server workflows, a client can still explicitly be passed in, or set in a local scope using withr syntax. For details of what this would look like, see below.

This change can be introduced without changing the signatures of existing functions, but it will enable us to introduce new functions that don't require a client argument.

Problem

Functions that operate on data that is not encapsulated in an R6 class need an argument for client. This limitation enforces some amount of clunkiness, which I'll do my best to explain.

Expressions that pipe function outputs are only possible when the functions produce an R6 object that contains a reference to client. For example, the piping workflows that feel "natural", e.g. in the Deployment section of the Pkgdown site, are limited to functions that return R6 objects that reference the client, otherwise, the code has no client available.

For example, content_item takes a client (connect) and a guid. Currently, you can write:

client |>
  content_item(GUID)

But if you have code that produces a relevant GUID, you can't place that function at the end of a pipe. The following will not work:

client |>
  get_content() |>
  pull(app_guid)[1] |> # Or any code that produces a GUID to operate on
  content_item() # Will not work, because `client` is required as the first argument.

Another way to think of the semantic problem is that, because all connectapi functions require access to the client object, you must currently always provide the client object itself or an R6 object that holds a reference to the client. We want to allow ourselves and our users the flexibility to build data pipelines that pull data from a Connect server, manipulate that data, and then interact with the Connect server again, taking the Connect client as a default piece of context for the code, and not having to explicitly pass it along at each step. (We definitely also want the ability to explicitly specify the client at any point too!).

This requirement also makes different approaches, which might diverge from the established R6 pattern, more tricky (e.g. get_job_log() in progress here).

The code we want is along the lines of:

client <- connect()
item <- content_item(client, GUID)
jobs <- get_jobs(content)
target_job <- jobs[[1]]
log <- get_job_log(target_job)

This code is only currently possible by making jobs contain a reference to the R6 client object. Without doing that, we're stuck with either get_job_log(content, job) or get_job_log(client, job).

Proposal

• Make a default client available in an environment at the top level.
  • Users can set this globally with set_default_client() and retrieve it with get_default_client().
  • Users can set scoped default clients, using with_client(client, code) and local_client(client, .local_envir) functions.
• The client argument to functions moves to the end of the argument list and becomes optional. (Also use this chance to standardize the name of this argument, which is sometimes connect, src, and client.)
  • When nothing is provided to that argument, the default client is used.
  • A Connect object can be provided to explicitly pass in a client.

Examples

Get the job log for a single content item. This code isn't possible today.

library(connectapi)

set_default_client(connect())

jobs <- get_content |>
  pull(app_guid)[1] |> # Get a single app GUID
  content_item() |>
  jobs()

log <- get_job_log(jobs[[1]])

Use different clients either by passing them to the client argument or by using withr-style syntax.

client1 <- connect(server = "http://connect1.example", api_key = Sys.getenv("KEY1"))
client2 <- connect(server = "http://connect2.example", api_key = Sys.getenv("KEY2"))

# Directly pass a client to a function's `.client` argument.
item1 <- content_item(GUID1, .client = client1)

# Use with_client(client, code) to set the default client for the execution of `code`
with_client(client2, {
  item <- content_item(GUID2)
})

# Manually set the default client and execute code without explicitly passing a client.
set_default_client(client1)
delete_vanity_url(GUID1)

# Use `local_client()` to set the default client for a local scope.
for (guid in c(GUID1, GUID2) {
  local_client(client2)
  delete_vanity_url(guid)
}

Implementation

(Taken from notes from a discussion with @schloerke a while back.)

Creating the environment where the default client lives; setting and getting the default client.

.connectapi_env <- new.env(parent = emptyenv())

set_default_client <- function(client) {
  old <- .connectapi_env$default_client
  .connectapi_env$default_client <- client
  invisible(old)
}

get_default_client <- function() {
  .connectapi_env$default_client
}

Implementation for withr-style functions.

with_client <- function(client, code) {
  old <- get_default_client()
  on.exit(set_default_client(old))
  set_default_client(client)
  force(code)
}

local_client <- function(client, .local_envir = parent.frame()) {
  old <- get_default_client()
  withr::defer(set_default_client(old), envir = .local_envir)
  set_default_client(client)
  invisible(old)
}

Internally, .client will default to NULL. That argument will be resolved by a resolve_client() function implemented like this.

content_item <- function(guid, ..., .client = NULL) {
  client <- resolve_client(client)

  res <- connect$get_connect()$content(guid)

  Content$new(connect = connect, content = res)
}

resolve_client <- function(client) {
  if (R6::is.R6(client) && !inherits(client, "Connect")) {
    client
  } else {
    get_default_client()
  }
}

Introduction & Adoption

I think it'd be good to introduce this convention sooner rather than later. It will allow us to maintain compatibility with existing R6 classes, while freeing us up to work with lighter-weight objects that only contain data returned by the server, whether those are new class paradigms or just lists returned by server endpoints.

We should introduce this in a minor update, without changing existing code — introducing this pattern does not mean that all functions in the package would need to change, just that we can use this pattern for newly introduced functions (e.g. get_job_log() linked above).

If we wanted to shift the entire package towards this pattern, I see two strategies that we could pursue:

1. In a major update, change all function signatures that currently take a client object.
2. Begin to introduce new functions with different signatures alongside old functions, using a different naming scheme such as object_verb().

Option (1) would be more work in the short term but likely less work in the long term, as we wouldn't need to reimplement every function. It would be more immediately disruptive to our customers, who would need to update existing code to take the new version of connectapi.

Option (2) would be more work, but the more gradual move would allow customers time to port code while still receiving package updates.

Misc Notes

It's worth noting that in many cases, passing around a reference to a client object is the correct thing to do, e.g. a class of object representing a content item should still contain a reference to its Connect server, as it's only meaningful in the context of that server.

[toph-allen]

Chatted with @schloerke, who added the suggestions that:

• The client argument should always come after ..., even if it's the only argument, so it must always be specified by name if provided.
• When thinking about a standardized name, we might want to name it .client

I'm amending some of the example code chunks with this in mind.