Enhancements to service construction?

[hadley]

Instead of

svc <- acm(
  config = list(
    credentials = list(
      creds = list(
        access_key_id = "string",
        secret_access_key = "string",
        session_token = "string"
      ),
      profile = "string"
    ),
    endpoint = "string",
    region = "string"
  )
)

Could we have this?

svc <- acm(
  credentials = list(
    creds = list(
      access_key_id = "string",
      secret_access_key = "string",
      session_token = "string"
    ),
    profile = "string"
  ),
  endpoint = "string",
  region = "string"
)

This makes life easier because auto-complete works, and you could document the individual parameters with a little more detail. You could make the change backward compatible by continuing to provide the config parameter and then combining it with the list generated by the other parameters.

It would also be useful if NULL values were automatically dropped, so that if you're wrapping in a function, you can default those arguments to NULL in your function.

[davidkretch]

Yeah, that sounds good. I think that should be pretty straightforward.

[DyfanJones]

@hadley @davidkretch sorry for taking so long to address this issue.

Original

svc <- acm(
  config = list(
    credentials = list(
      creds = list(
        access_key_id = "string",
        secret_access_key = "string",
        session_token = "string"
      ),
      profile = "string"
    ),
    endpoint = "string",
    region = "string"
  )
)

Proposal 1

As suggested by @hadley

svc <- acm(
  credentials = list(
    creds = list(
      access_key_id = "string",
      secret_access_key = "string",
      session_token = "string"
    ),
    profile = "string"
  ),
  endpoint = "string",
  region = "string"
)

My concerns with this approach would be:

• breaking change to dependent packages
• anytime we do an update to the Config struct in paws.common for new parameters we will need to do a paws regen to pick up the new parameter. For example PR https://github.com/paws-r/paws/pull/636 adds sts_regional_endpoint.

However we could get around this by keeping the config parameter plus it would prevent breaking dependent packages.

# modified to keep the config parameter
svc <- acm(
  credentials = list(
    creds = list(
      access_key_id = "string",
      secret_access_key = "string",
      session_token = "string"
    ),
    profile = "string"
  ),
  endpoint = "string",
  region = "string",
  config = list()
)

Proposal 2

svc <- acm(config = list(), ...)

This allows the config parameters to be called at "root" for example:

svc <- acm(
  access_key_id = "string",
  secret_access_key = "string",
  session_token = "string",
  profile = "string",
  endpoint = "string",
  region = "string"
)

The down side to this it doesn't address the auto-complete functionality, but it does make initialising the service a little easier. I have a PR https://github.com/paws-r/paws/pull/639 toying with this idea.

What's everyone view on this?

[hadley]

I think you should keep the config argument, and just combine with credentials, endpoint, etc. I don't think there's much benefit to using ... in this way, as it's hard to document.

[DyfanJones]

Ok the only issue we have now is credentials is a list parameter and is doesn't support auto-compelete. After chatting with @davidkretch paws.common has a nifty class called struct that does help with auto-complete

get_service_parameter <- function(param) {
  firstup <- function(x) {
    substr(x, 1, 1) <- toupper(substr(x, 1, 1))
    x
  }
  fn <- get(param, envir = getNamespace("paws.common"))
  elements <- fn()
  elements <- elements[grep("provider", names(elements), perl = T, invert = T)]
  
  for (nms in names(elements)) {
    if (is.list(elements[[nms]])) {
      elements[[nms]] <- get_service_parameter(firstup(nms))
    }
  }
  return(elements)
}

build_service_parameter <- function(param) {
  return(
    do.call(
      struct,
      get_service_parameter(param)
    )
  )
}

SetConfig <- build_service_parameter("Config")
SetCredentials <- build_service_parameter("Credentials")
SetCreds <- build_service_parameter("Creds")

It will also allow for mix lists and structs together

Also it is handy when they are nested together.

I believe these could be possible helper functions.

What is everyones thoughts on this?

[DyfanJones]

Update of progress:

The helper functions will snake case to be consistent with the rest of paws exported functions and methods.

Short term the helper functions can be accessed by loading paws.common into session.

library(paws.common)
s3 <- paws::s3(config(credentials(profile = "paws")))
s3$list_buckets()

Long term we would want to re-export the helper functions to all paws service packages so that paws.common doesn't need to loaded explicitly. Plus in combination with the extra service parameters.

s3 <- paws::s3(config(credentials(profile = "paws")))
s3$list_buckets()

# or

s3 <- paws::s3(credentials = credentials(profile = "paws"))
s3$list_buckets()

# or

s3 <- paws::s3(credentials = list(profile = "paws"))
s3$list_buckets()

[DyfanJones]

paws v-0.4.0 has now been released to the cran. I will close this ticket for now.