Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thoughts on spaces API integration #136

Closed
rpodcast opened this issue Sep 1, 2017 · 18 comments
Closed

Thoughts on spaces API integration #136

rpodcast opened this issue Sep 1, 2017 · 18 comments
Labels
Milestone

Comments

@rpodcast
Copy link
Contributor

rpodcast commented Sep 1, 2017

I'm a big fan of analogsea as digital ocean is a huge part of my infrastructure, so thank you so much for creating this package! Recently digital ocean has begun rolling out early access to their new object storage capability (also known as spaces) and I'm fortunate enough to be part of the early beta for access. It operates in a similar way to amazon S3; in fact they tried to make their spaces API quite compatible with the amazon S3 api. I see great potential for this feature and would be glad to work with you on implementing this in analogsea if you are interested.

@sckott
Copy link
Collaborator

sckott commented Sep 5, 2017

@Thercast Glad you like the package.

This sounds good to me! I just applied for it as well so that I can contribute to development

How do you want to proceed? Do you have time to work on the code for this, or do you want me to start on it? If you start on it, do follow general style we've used in this package

@rpodcast
Copy link
Contributor Author

rpodcast commented Sep 6, 2017

Thanks @sckott! I am interested in providing development help (I've been looking to contribute to R packages that wrap APIs) but wanted to get your opinion on whether we should try to leverage some of the functions from the aws.s3 package, given the compatability of the API? I will investigate aws.s3 in more detail to get a better idea of the functionality it offers.

@sckott
Copy link
Collaborator

sckott commented Sep 6, 2017

okay

@sckott
Copy link
Collaborator

sckott commented Sep 26, 2017

any thoughts on this?

@amoeba
Copy link
Contributor

amoeba commented Sep 26, 2017

[Commenting because I had been lurking on this thread for a few weeks] I'd be down with helping implement this if the help was needed.

@rpodcast
Copy link
Contributor Author

rpodcast commented Sep 26, 2017 via email

@sckott
Copy link
Collaborator

sckott commented Sep 26, 2017

@Thercast i think you meant to put that in discgolf?

@rpodcast
Copy link
Contributor Author

rpodcast commented Sep 26, 2017 via email

@sckott
Copy link
Collaborator

sckott commented Sep 26, 2017

@amoeba i think so, maybe wait to see first PR from @Thercast then divy up work


PR's sounds good

@amoeba
Copy link
Contributor

amoeba commented Sep 27, 2017

Sounds good to me!

@rpodcast
Copy link
Contributor Author

@sckott I have preliminary support for a few of the key operations for spaces (listing spaces, listing objects within a space, and creating a new space) ready for your review at this custom branch. I have more polishing to do but didn't want to go too far in case you have different ideas.

It turns out that the aws.s3 package functions work nicely with digital ocean spaces once you set a few of the parameter values differently (such as base url and the access/secret keys). It does bring a few additional package imports (base64enc, digest, aws.signature) so that is something to consider. Also I acknowledge that these new functions stick out in terms of their overall methodology (i.e. no S3 methods/classes). If you like the general direction I'll be happy to add some additional operations (such as deleting spaces and adding/deleting objects within spaces).

@sckott
Copy link
Collaborator

sckott commented Sep 28, 2017

@Thercast

  • are the key and secret for spaces different than for droplets? looks like it, just making sure
  • i'd prefer not to have users pass in access key and secret but to instead have them set env vars
  • i think going with aws.s3 is good for now. nice that it works mostly out of the box. i do want to swap out httr for crul at some point, so may need to take it out, but good for now.
  • i do want to change returned objects from fxns to be similar to what this pkg does already, e.g., the returned objects here https://github.com/sckott/analogsea#get-droplets - i think it's important to be consistent within this pkg
  • go ahead and send a PR whenever, can always add to it/chagne it

@rpodcast
Copy link
Contributor Author

@sckott here are some notes:

are the key and secret for spaces different than for droplets? looks like it, just making sure

Yes that's correct. Digital Ocean allows you to create multiple spaces keys, so you can specify the key associated with a bucket during creation.

i'd prefer not to have users pass in access key and secret but to instead have them set env vars

I agree. One thing I forgot to mention is that the functions will look for environment variables DO_SPACES_ACCESS_KEY and DO_SPACES_SECRET_KEY in the session if they are not passed explicitly in the function calls. If they are not defined, then an error message appears. I defined those in ~/.Renviron in my testing.

i do want to change returned objects from fxns to be similar to what this pkg does already, e.g., the returned objects here https://github.com/sckott/analogsea#get-droplets - i think it's important to be consistent within this pkg

Makes perfect sense. With a new space object type, we can have similar methods for summary and print like the droplet object/methods. Some useful stats (like total space size) aren't exposed directly in the API, hence many s3 clients need to grab the info for all objects and then sum their file size. For small spaces that will still be manageable, but I tested with one of my spaces containing 17k objects and space('name_of_space') took approx 15 seconds.

go ahead and send a PR whenever, can always add to it/chagne it

I'll do a bit more cleanup (documentation and adding tests) and put together basic print and summary methods for spaces much like your existing methods for droplets before I send a PR.

@sckott
Copy link
Collaborator

sckott commented Sep 29, 2017

All sounds good

@sckott
Copy link
Collaborator

sckott commented Oct 17, 2017

Now that #138 is merged, can you two divy up tasks ti fill out remaining methods for spaces API?

@amoeba
Copy link
Contributor

amoeba commented Oct 17, 2017

Glad to hear it. Nice work @Thercast! @Thercast do you want to make a new Issue with a checklist of remaining tasks and I can put my name on a few if you want help?

@rpodcast
Copy link
Contributor Author

@amoeba that sounds like a great idea. Please go ahead and create a new issue and we can discuss how to divy up the remaining pieces.

@sckott
Copy link
Collaborator

sckott commented May 19, 2018

closing in favor of continuing discussion in #140

@sckott sckott closed this as completed May 19, 2018
@sckott sckott modified the milestones: v0.7, v0.8 Jun 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants