Add support for basic digital ocean spaces API #138

rpodcast · 2017-10-07T04:23:57Z

Description

This PR brings preliminary support for utilizing the digital ocean spaces object storage API. The spaces API is backward-compatible with the Amazon S3 object storage API, so I leverage key functions from the aws.s3 package to work with the API. Since the authentication is controlled by separate keys from the normal digital ocean API, the user is expected to create these keys within their DO spaces account, and the user can pass the key ID and secret directly in the functions or define them as environment variables (DO_SPACES_ACCESS_KEY and DO_SPACES_SECRET_KEY). The main functions introduced are the following:

spaces() : Obtain all spaces in your account. Each one will be given a class called space with custom print and summary S3 methods.
spaces_GET() : Supporting function that wraps aes.se::s3HTTP() to obtain the spaces information using the API.
space_info() : Obtain metadata around all files contained in a space. Note that if the space contains more than 1,000 files that this operation can take a bit of time, since the S3 API only allows for up to 1,000 files to be queried at a time.

Related Issue

#136

Example

# obtain the spaces in your DO account
my_spaces <- spaces()

# print summary information of the space called "test-space"
summary(my_spaces[["rpodcast-space3"]])

<space_detail>rpodcast-space3
  Size (GB):     37.54697 
  Files:        17331
  Created at:    2017-08-27T19:01:05.737Z 

# create a new space in your account
space_create("my-new-space")

update to upstream

… a space, and creating new spaces.

sckott

thanks for this!

have some questions in line, and also:

would be good to add some examples in the documentation for use of each fxn

sckott · 2017-10-07T19:39:19Z

R/spaces.R

+
+  res <- put_bucket(name,
+             region = NULL,
+             acl = acl,


this acl thing is undefined, what is it supposed to be?

That is one of the parameters used by put_bucket function from aws.s3 to determine if the bucket is made private or public upon creating. On default new buckets are made as private, and it appears to also be true when I use it to createi new spaces. I tried specifying this to be public in my testing but the spaces were still being created as private. Until I can get this resolved I will remove references to it.

sounds good, thx

sckott · 2017-10-07T19:40:27Z

R/spaces.R

+             base_url = spaces_base,
+             ...)
+
+  if (res) message(sprintf("New space %s created successfully"))


doesn't sprintf expect an object passed to it?

Good catch, I will fix that. It was supposed to be passing name. That's what I get for coding late 😞

sckott · 2017-10-07T19:43:24Z

R/spaces.R

+#' @importFrom aws.s3 bucketlist
+#' @export
+#' @rdname spaces
+spaces_GET <- function(spaces_key = NULL, spaces_secret = NULL, ...) {


did you mean to export spaces_GET ? if so, what does it do? from the name of the fxn it seems like it's meant to be an internal fxn maybe

This was also supposed to be an internal function, since the spaces() function is the one intended to be user facing. I'll make sure this is internal in my revisions.

sckott · 2017-10-07T19:48:27Z

R/spaces.R

+             base_url = spaces_base,
+             ...)
+
+  if (res) message(sprintf("New space %s created successfully"))


Will this error well if the space isn't created? curious what that looks like. i assume that error handling is in aws pkg

So far I haven't had any errors occur when creating a new space. But I am going to add a simple check to ensure the user does not have an existing space with the same name.

okay, something to explore later, though may be handled well already in aws.s3 pkg

sckott · 2017-10-07T19:59:37Z

R/spaces.R

+#' Spaces storage operations
+#'
+#' \describe{
+#'  \item{space}{List space contents}


i don't see a space function, though maybe it's something to be added

ah, maybe space == space_info

I went back and forth with this, but now I'm going to omit a space() function since spaces() will grab everything and it is straight forward for the user to grab whichever space they want to work with. They all have a special space class so we'll be able to add many more methods and operations as time goes on I'll delete references to a space function in my next PR.

Also I've made space_info() an internal function

sounds good

…ount

rpodcast · 2017-10-08T18:38:33Z

Thanks for your review @sckott! I've added comments above and pushed the updates.

sckott · 2017-10-10T03:11:19Z

It's not a great idea to use internal variables that are the same name as the function they are within, e.g,. spaces within spaces function

sckott

A few more comments. also:

i haven't done it in every file yet, but we try to keep to 80 chars in width for docs/comments/code

sckott · 2017-10-10T03:12:36Z

R/spaces.R

+#' @rdname spaces
+spaces <- function(spaces_key = NULL, spaces_secret = NULL, ...) {
+  res <- spaces_GET(spaces_key = spaces_key, spaces_secret = spaces_secret, ...)
+  spaces <- lapply(res$Buckets, structure, class = "space")


try doing unname(res$Buckets) because if you have only 1 item returned, it fails, try it out you'll see what i mean

This was rather nefarious, but when only one space is present the structure of the result changed such that only the Name and CreationDate are populated in res$Buckets, instead of each space occupying a Bucket object (list) containing the Name and CreationDate. I've added a check of this via 35ff528.

sckott · 2017-10-10T03:21:31Z

R/spaces.R

+  spaces_secret <- check_space_secret(spaces_secret)
+
+  # check if space name already exists in user's account
+  spaces_existing <- spaces()


i think i might rather have the digital ocean API handle this than us handle it client side. that is, they handle both cases where a name already exists in the users set of spaces or anyone else's spaces - one reason is to have consistent error behavior, another is that the function will be faster, as it's one less HTTP request

That's a good point, why make additional HTTP requests when it's not necessary. I was initially put off by the verbose printing of the result by aws.s3, but it is getting the right status and message in these situations. I removed my manual check via cea751a.

sckott · 2017-10-10T03:48:58Z

R/spaces.R

+#' @importFrom aws.s3 put_bucket
+#' @export
+#' @rdname spaces
+space_create <- function(name, spaces_key = NULL, spaces_secret = NULL, ...) {


instead of name with no default, you could do https://github.com/sckott/analogsea/blob/master/R/droplet-actions.R#L84

don't know if that makes sense for spaces, def. feel free to not change it

I'm inclined to not give name a default because like Amazon S3, a space name must be unique across all spaces defined in digital ocean, not just the spaces within a user's account. While the actual chance of users getting the same randomly generated name are quite small, I'm still weary of generating a random name as default.

…t as when two or more spaces are present

rpodcast · 2017-10-12T02:54:16Z

I finally got a chance to make updates and address your latest comments. Please take a look and let me know if you see other areas that need revisions.

sckott · 2017-10-17T17:26:59Z

Looks good @Thercast

Let's merge this, then we can go back to #136 and discuss how to divy up tasks

rpodcast added 8 commits September 24, 2017 13:15

Merge pull request #1 from sckott/master

7be15c2

update to upstream

Merge branch 'master' of github.com:sckott/analogsea

6fedc20

add aws.s3 package to imports

8e1661d

preliminary support for listing all spaces in account, objects within…

991bff6

… a space, and creating new spaces.

redo space functions to use a consistent s3 object paradigm

aee4cb0

polish documentation

7233bb7

add test for space_GET()

9c37877

fix tests for spaces functions

d356f71

sckott requested changes Oct 7, 2017

View reviewed changes

rpodcast added 5 commits October 8, 2017 13:08

add method to convert string of space name to a space object

18f99fa

clean up spacing for print method

5e5ffb5

make space_info internal

110c188

ensure that space name requested does not already exist in user's acc…

644da88

…ount

regenerate documentation for space functions

af08b76

sckott reviewed Oct 10, 2017

View reviewed changes

rpodcast added 6 commits October 11, 2017 21:37

change name of tmp object in spaces function

44e5e5c

ensure that one space in account gives same structure of result objec…

35ff528

…t as when two or more spaces are present

remove manual check of existing space names when creating new space

cea751a

tidy up long lines

ea83011

update error message in test of spaces secret key

f08b373

tidy long lines in spaces tests

ceed414

sckott merged commit b55f1e3 into pachadotdev:master Oct 17, 2017

sckott added this to the v0.7 milestone Oct 17, 2017

sckott mentioned this pull request Oct 17, 2017

Thoughts on spaces API integration #136

Closed

amoeba mentioned this pull request Oct 18, 2017

Finish implementation of Spaces API #140

Open

27 tasks

sckott added the spaces label May 19, 2018

sckott modified the milestones: v0.7, v0.8 Jun 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for basic digital ocean spaces API #138

Add support for basic digital ocean spaces API #138

rpodcast commented Oct 7, 2017

sckott left a comment

sckott Oct 7, 2017

rpodcast Oct 8, 2017

sckott Oct 10, 2017

sckott Oct 7, 2017

rpodcast Oct 8, 2017

sckott Oct 7, 2017

rpodcast Oct 8, 2017

sckott Oct 10, 2017

sckott Oct 7, 2017

rpodcast Oct 8, 2017

sckott Oct 10, 2017

sckott Oct 7, 2017

sckott Oct 7, 2017

rpodcast Oct 8, 2017

rpodcast Oct 8, 2017

sckott Oct 10, 2017

rpodcast commented Oct 8, 2017

sckott commented Oct 10, 2017

sckott left a comment

sckott Oct 10, 2017

rpodcast Oct 12, 2017

sckott Oct 10, 2017

rpodcast Oct 12, 2017

sckott Oct 10, 2017

sckott Oct 10, 2017

rpodcast Oct 12, 2017

sckott Oct 17, 2017

rpodcast commented Oct 12, 2017

sckott commented Oct 17, 2017

Add support for basic digital ocean spaces API #138

Add support for basic digital ocean spaces API #138

Conversation

rpodcast commented Oct 7, 2017

Description

Related Issue

Example

sckott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpodcast commented Oct 8, 2017

sckott commented Oct 10, 2017

sckott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpodcast commented Oct 12, 2017

sckott commented Oct 17, 2017