Implement Object operations for Spaces API #146

amoeba · 2018-01-09T06:50:06Z

Description

This implements the first half of the operations listed in #140 which are the Spaces API operations related to Objects

Related Issue

#140

No tests yet, since all these tests require external calls. See discussion in #140

sckott

nice work.

Add yourself as author to DESCRIPTION file
pulled your branch down - looks like man/spaces_delete_object.Rd was updated, run document() or equiv. again and push that

sckott · 2018-01-09T18:22:16Z

R/spaces.R

+#'   with the spaces key. If missing, defaults to value stored in an environment
+#'   variable \code{DO_SPACES_SECRET_KEY}.
+#' @param ... Additional argument passed to \code{\link[aws.s3]{get_object}}
+#'


@amoeba if you want to include @author <youremail> tags for the functions you write go ahead

Fair enough. I'm okay not doing this.

sckott · 2018-01-09T18:44:20Z

R/spaces.R

+#' # basename
+#' spaces_put_object("./some-file.txt", space = "my-space")
+#' }
+spaces_put_object <- function(file, object = basename(file), space, spaces_key = NULL, spaces_secret = NULL, ...) {


this function doesn't error well if the file doesn't exist - maybe an internal check for that?

sckott · 2018-01-09T18:45:31Z

R/spaces.R

+#' @examples
+#' \dontrun{
+#' spaces_put_object("./some-file.txt", "my-object", "my-space")
+#' spaces_get_object("my-object", "my-space")


adding an example of how to convert the raw bytes to text would be good, e.g, rawToChar(x)

Great suggestion! Will do.

sckott · 2018-01-09T18:49:25Z

R/spaces.R

+#' spaces_put_object("./some-file.txt", "my-object", "my-space")
+#' spaces_head_object("my-object", "my-space")
+#' }
+spaces_head_object <- function(object, space, spaces_key = NULL, spaces_secret = NULL, ...) {


Toughts on the output structure from this function? Maybe make it into a named list instead? just via attributes(x)

I like that.

Now also shows the user an example of converting the response to its character representation

amoeba · 2018-01-11T06:19:25Z

Thanks for the comments. Will update this PR.

Since this is a call to a non-exported s3 method from xml2 and aws.s3 imports xml2, I think we're good in terms of this working as intended

sckott · 2018-01-23T19:22:40Z

@amoeba sorry for delay - getting to looking this over hopefully today

sckott · 2018-01-25T02:27:17Z

DESCRIPTION

@@ -10,6 +10,8 @@ Authors@R: c(
    person("Hadley", "Wickham", role = "aut", email = "hadley@rstudio.com"),
    person("Winston", "Chang", role = "aut", email = "winston@stdout.org"),
    person("Bob", "Rudis", role = "ctb", email = "bob@rudis.net"),
+    person("Bryce", "Mecum", role = "ctb", email = "brycemecum@gmail.com",
+           comment = c("ORCID" = "0000-0002-0381-3766")),


nice, will gather ORCIDs for others later

sckott · 2018-01-25T02:28:57Z

R/spaces_acl_get.R

+                              base_url = spaces_base,
+                              ...)
+
+  # if (!requireNamespace("xml2")) {


@amoeba i forget, did you mean to leave this block commented out?

Whoops. I did not! Will fix.

sckott · 2018-01-25T02:45:39Z

@amoeba is space_create working for you?

I'm getting an error on every try, checked that my env vars are there.

SO with verbose curl output i'm seeing

space_create('new_space_name', config=httr::verbose())
-> PUT /new_space_name/ HTTP/1.1
-> Host: nyc3.digitaloceanspaces.com
-> User-Agent: libcurl/7.54.0 r-curl/3.1 httr/1.3.1
... cut out some stuff
<- HTTP/1.1 400 Bad Request
<- Date: Thu, 25 Jan 2018 02:37:06 GMT
<- Content-Length: 153
<- Content-Type: text/plain; charset=utf-8

so the URL seems like nyc3.digitaloceanspaces.com/space-name

whereas in creating one in the DO platform, and the API docs say the same, looks like the URL should be of the form

space-name.nyc3.digitaloceanspaces.com

thoughts? does it work for you?

sckott · 2018-01-25T02:52:29Z

R/spaces_acl_put.R

+#' spaces_object_put("some-file.txt", space = "my-space")
+#'
+#' # You can get a copy of the ACL
+#' acl <- spaces_acl_get("some-file.txt", "my-space") %>% read_xml()


in examples, with Suggested pkgs, should use them conditionally on whether user has the pkg.

so

if (requireNamespace("xml2")) { do the thing }

and inside that block (if user has xml2 installed) namespace xml2 calls

Oh, interesting. I can certainly do that.

sckott · 2018-01-25T02:53:33Z

R/spaces_acl_put.R

+#' acl <- spaces_acl_get("some-file.txt", "my-space") %>% read_xml()
+#'
+#' # Modify it as needed, then update it
+#' spaces_acl_put("my-object", "my-space", acl)


is my-object supposed to be equivalent to some-file.txt ? if so could you change that, so peole can actually do the eg

That was a mistake. Fixed.

sckott · 2018-01-25T02:59:43Z

I couldn't get spaces_object_delete to work. I kept getting a The specified bucket does not exist error, even though the bucket defiinitely existed. work for you?

amoeba · 2018-01-25T05:31:01Z

Thanks for taking a look. I really appreciated the letting us have a go at this and the helpful comments.

I don't think I have ever been able to space_create working. I assumed it was just still in progress. But I think spaces_object_delete was working fine. I'll test now.

amoeba · 2018-01-25T05:36:10Z

Hrm, spaces_object_delete worked fine for me just now:

> spaces_object_delete("GDP.csv", "test-analogsea")
[1] TRUE

sckott · 2018-01-25T05:42:49Z

Glad delete works. Wonder if they changed how spaces create works since that fxn was originally made here?

amoeba · 2018-01-25T05:44:56Z

I wonder. I'll take a look at it.

amoeba · 2018-01-25T05:58:58Z

Looks like a limitation of aws.s3. In setup_s3_url, which appears to generate the URLs used in put_bucket (which creates new S3 buckets), it hard-codes this logic:

# handle S3-compatible storage URLs
    if (base_url != "s3.amazonaws.com") {
        if (isTRUE(verbose) && url_style != "path") {
            message("Non-AWS base URL requested. Switching to path-style URLs.")
        }
...

which looks like the cause of our problem here.

sckott · 2018-01-25T06:18:51Z

ah, that could be it, right, i assume then it adds the bucket/space name after the base url

amoeba · 2018-01-25T06:43:50Z

yup

sckott · 2018-02-07T19:30:12Z

bump @amoeba , are you waiting on me to do anything here?

amoeba · 2018-02-07T19:45:55Z

Hey @sckott thanks for the bump. This can be merged if you think so. I looked over all of the comments here and in the related issue and I think it's good to go.

sckott · 2018-02-07T20:03:16Z

okay, will take another quick look

sckott · 2018-02-07T20:21:38Z

@amoeba can you add man-roxygen to .Rbuildignore

amoeba · 2018-02-07T20:27:21Z

Done!

sckott · 2018-02-07T20:28:54Z

thanks for the rbuildignore fix.

hmm, did you see my comment about space_create not working? #146 (comment)

amoeba · 2018-02-07T20:36:37Z

I did. I was thinking that, since this PR is just for the Object API operations that I/we'd work on that elsewhere. It'll require either an upstream aws.s3 fix or moving away from using aws.s3. Is that okay?

sckott · 2018-02-07T20:42:50Z

right, forgot about that. yep, we'll deal with fixing create later.

amoeba added 2 commits January 8, 2018 21:45

Implement Object operations for Spaces API

b046c72

Fix broken link in spaces_delete_object docs

1799881

amoeba mentioned this pull request Jan 9, 2018

Finish implementation of Spaces API #140

Open

27 tasks

sckott reviewed Jan 9, 2018

View reviewed changes

amoeba added 3 commits January 10, 2018 21:10

Expand examples in spaces_put_object

2469cb6

Now also shows the user an example of converting the response to its character representation

Change spaces_head_object's return to a named list

f692025

Add stop() when file arg in spaces_put_object isn't found

de52295

amoeba added 11 commits January 10, 2018 21:24

Rename Spaces Object functions and move into separate files

4979026

Hard wrap Spaces Object source to 80 cols for consistency

7df4cf3

Rename spaces_object_acl_* to spaces_acl_* as per scott's idea

ce3b440

Wrap booleans in \code{} blocks in docs as per scott

16d3701

Overhaul examples for Object functions per scott

0eec465

Use a roxygen template for space key/secret @params

772a76a

Add myself to DESCRIPTION file

be7b5b3

Add support for parsing/serialzing Object ACLs with package xml2

19a98b0

Fix roxygen error due to a use of #'

7bb7099

Add note in spaces_object_put examples about multipart

70dd1da

Remove explicit ::: call to as.character.xml_document

2be8bfc

Since this is a call to a non-exported s3 method from xml2 and aws.s3 imports xml2, I think we're good in terms of this working as intended

sckott reviewed Jan 25, 2018

View reviewed changes

amoeba added 2 commits January 24, 2018 20:36

Uncomment code that shouldn't've been commented

1b39b4a

Redo spaces_acl_put examples to fix mistakes

51127bf

Add man-roxygen to Rbuildignore as per sckott

8a506f9

sckott added this to the v0.7 milestone Feb 7, 2018

sckott added the spaces label Feb 7, 2018

sckott merged commit 597484d into pachadotdev:master Feb 7, 2018

amoeba deleted the spaces-object-api branch February 7, 2018 20:49

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

Implement Object operations for Spaces API #146

Implement Object operations for Spaces API #146

Conversation

amoeba commented Jan 9, 2018

Description

Related Issue

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

amoeba commented Jan 11, 2018

sckott commented Jan 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sckott commented Jan 25, 2018

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

sckott commented Jan 25, 2018

amoeba commented Jan 25, 2018

amoeba commented Jan 25, 2018

sckott commented Jan 25, 2018

amoeba commented Jan 25, 2018

amoeba commented Jan 25, 2018

sckott commented Jan 25, 2018

amoeba commented Jan 25, 2018

sckott commented Feb 7, 2018

amoeba commented Feb 7, 2018

sckott commented Feb 7, 2018

sckott commented Feb 7, 2018

amoeba commented Feb 7, 2018

sckott commented Feb 7, 2018

amoeba commented Feb 7, 2018

sckott commented Feb 7, 2018