storage: storage.createBucket('name', cb)

[ryanseys]

Our README says that we must create a bucket on the developers console before we can use it in this library. Can't we provide a method to allow developers to create buckets on the fly?

var gcloud = require('gcloud');
var storage = gcloud.storage;

storage.createBucket('ryans-selfies', function(err) {
  console.log(err || 'You may now upload your selfies.');
});

[stephenplusplus]

This makes sense to me. Is there any reason we can't do this @silvolu?

https://developers.google.com/storage/docs/json_api/v1/buckets/insert

[rakyll]

We should also provide listing, deletion, etc. Adding this to M2.

[stephenplusplus]

That's a good point. What API can we use that makes sense for that?

The following suggestion would require creating a Buckets instance, which is how you get through to an individual bucket, of the current type Bucket.

var storage = gcloud.storage;

var buckets = new storage.Buckets(/*credentials*/);
// buckets instanceof Buckets

buckets.list(function(err, buckets) {
  // buckets = Bucket[]
});

buckets.get('Photos', function(err, photos) {
  // photos instanceof Bucket
});

buckets.create('Albums', function(err, albums) {
  // albums instanceof Bucket
});

new storage.Buckets may sound a little weird, but I made it this way for these reasons:

1. Our other APIs follow the new instantiation requirement

2. We shouldn't have to provide connection details to more than one bucket. By defining this once when we create a Buckets instance, we don't have to worry about this. As opposed to...

   var photos = new Bucket({ credentials: {} });
   var albums = new Bucket({ credentials: {} });

[ryanseys]

I'm not a big fan of new in API clients (take a look at google-api-nodejs-client and you'll see I try to avoid it).

It does make sense to specify options only once so I think we should do that. But what about even sooner than Buckets? The credentials themselves should work for all gcloud apis, right?

So what about something radical like:

var gcloud = require('gcloud');

var client = new gcloud.Client({ credentials: /* ... */ });
// or just gcloud.options({ credentials: /* ... */ });

var storage = client.storage;
// or just gcloud.storage if we use .options() example above

storage.buckets.list(function(err, buckets) {
  var photos = buckets.get('Photos');
  if(!photos) {
    buckets.create('Photos', function(err, photos) {
      photos.upload('funnycat.gif', { /* opts */ }, function(err) {
        console.log(err || 'Uploaded funny cat gif');
      });
    });
  }
});

What calls need to be async here? buckets.create('Albums') sounds like it needs to be async.

[stephenplusplus]

I'm not a big fan of new in API clients (take a look at google-api-nodejs-client and you'll see I try to avoid it).

You won't catch it in any of my stuff, either. I would like to argue that new doesn't make sense here, but I just can't. We're creating multiple instances of an object in the most light-weight way possible. This isn't to say it's right for my theory API above, but in general, I support it where we use it in this library.

But what about even sooner than Buckets?

If we can do this, I like this much better. I'll have to read through this again for a refresher, but #46 (comment) was a blocker.

What calls need to be async here? buckets.create('Albums') sounds like it needs to be async.

create and get from my example should have been async. I've updated it.

[ryanseys]

I'm not saying using new doesn't make sense because it totally does, but I don't want the end-developer to worry about using it. new is used in the other client to create every endpoint but that is hidden, and what's exposed to the developer is simply var drive = google.drive('v2'); instead of var drive = new google.Drive('v2');. I should be able to call on the api by just passing in names and ids, not creating objects and 'saving' them or doing weird operations on them. It should be as easy as:

require('gcloud')
  .options({ credentials: require('./key.json') })
  .storage
  .createBucket('bucket name', callback);

[stephenplusplus]

The more you hate on it, the more I agree with you. It feels like forcing the developer to use new is important for them to understand "you're getting a new object that is meant to be multiply instantiated," but I suppose you can argue that that's an implementation detail of how our internal structure works, and saying gcloud.datastore.dataset(), for example, can be just as clear, especially with the help of the documentation.* I'll recap this in #172, then we can seek opinions of others if and when we can implement it.

require('gcloud')
  .options({ credentials: require('./key.json') })

This stuff should probably be talked about in a separate thread (perhaps the one I linked before). For now, we should just talk about how we can implement the Buckets api in our current structure. If and when we can support something like the above example, that will require a whole revamp of our code anyway.

* Sorry for the long sentence, but if anyone deserves that, it's you :P (just kidding of course)

[ryanseys]

Haha sorry, I ramble sometimes. But I really want this API client to be something we don't even have to teach developers to use. For storage, there's only a few key concepts we need to teach them: auth, buckets, files. Then the documentation will show them all the methods they can run on those concepts i.e. list buckets, upload file, list files, delete bucket, etc..

We can give them a couple examples and they can run with it. I want developers to be able to correctly GUESS how our client works, that's how easy it should be 😄 It's a lot to ask, but I think we can do it.

Current example (from our docs):

var gcloud = require('gcloud');
var storage = gcloud.storage;
var bucket;

// From Google Compute Engine:
bucket = new storage.Bucket({
  bucketName: YOUR_BUCKET_NAME
});

// Or from elsewhere:
bucket = new storage.Bucket({
  bucketName: YOUR_BUCKET_NAME,
  keyFilename: '/path/to/the/key.json'
});

bucket.write('demo.txt', 'Hello World', function(err) {
  console.log(err || 'Created demo.txt');
});

Could be simplified to:

var gcloud = require('gcloud');
gcloud.options({ keyFilename: !ON_COMPUTE && '/path/to/the/key.json' });
// some env variable used here to detect if you're on GCE

var storage = gcloud.storage;

storage.bucket('ryans-notes').write('demo.txt', 'Hello World', function(err) {
  console.log(err || 'Created demo.txt');
});

[ryanseys]

As an aside, if we made these changes, it would start to look and act a lot more like google-api-nodejs-client, making a developer's transition from one library to another less painful.

This library has the added benefit that we can hand-craft the api structure instead of having to tweak a bunch of templates and push it through a code generator (which is a huge pain the ass and requires a lot design upfront). I'm trying to bring those "design upfront" ideas here because it helps save a lot of time but we can still iterate and tweak as needed with more flexibility.

(That was a long aside) 😳

[stephenplusplus]

I see this as two separate discussions. Part 1 being allowing creation of a connected gcloud object, and Part 2 being how we can expand the Storage api we offer. I think it would make sense to continue Part 1 in the place where it left off: #46 and we can put Part 2 on hold if you think Part 1 needs to be resolved first.

storage.bucket('ryans-notes').write('demo.txt', 'Hello World', function(err) {
  console.log(err || 'Created demo.txt');
});

I really like the simplicity here. This way behaves as it does currently, as far as waits for the call to write to see that it would throw if that bucket doesn't exist. And that's ok, if we're ok with that. But, I was starting to like what we were working towards earlier in this thread, by getting that error immediately before thinking that you can get away with a write:

// assuming `stephen-notes` doesn't exist...
var bucket = storage.bucket('stephen-notes');
['things', 'to', 'write', 'to', 'my', 'secret', 'journal'].forEach(function(note) {
  bucket.write(note, function(err) {
    // 7 api calls and fails.
  });
});
// vs..
storage.bucket('stephen-notes', function(err, bucket) {
  if (err) {
    // don't waste time thinking bucket operations will work
    return;
  }
  // make api calls that will work.
});

Any thoughts on all that? Should we allow assuredness while getting a bucket that it exists, or is it ok to continue to wait until the API calls?

As a note, when we allow creation of buckets, what method name should we use? storage.bucket doesn't make it clear to me if I'm creating or getting.

[ryanseys]

I see this as two separate discussions.

Opened #191 for gcloud.options()

by getting that error immediately before thinking that you can get away with a write

Are you saying that new storage.Bucket({ bucketName: 'ryan-notes' }); would make a request to the API synchronously? You seem to be implying there is checking currently but I can't see that being the case.

Should we allow assuredness while getting a bucket that it exists, or is it ok to continue to wait until the API calls?

Can we support both? If you supply a callback, make the request and callback a success/fail. Don't supply a callback, then assume it exists and create the bucket only locally.

We can also supply methods to check if a bucket exists / create a bucket so they can do the callback step themselves. Then storage.bucket(name) is just way to define what bucket we want to perform actions on, and yes we will receive errors if the bucket doesn't exist or they don't have permission.

storage.bucket('stephen-notes').exists(function(nope) {
  if(nope) {
    storage.bucket('stephen-notes').create(function(err, created) {
      storage.bucket('stephen-notes').upload(filename, callback);
    });
  } else {
    storage.bucket('stephen-notes').upload(filename, callback);
  }
});

[stephenplusplus]

Are you saying that new storage.Bucket({ bucketName: 'ryan-notes' }); would make a request to the API synchronously?

Yeah, to verify the bucket exists, and prevent future api calls to fail. It's going to fail eventually, might as well fail when the user tries to access a bucket is my thought, so they can address the issue before making possibly multiple failure calls to write and other methods.

You seem to be implying there is checking currently but I can't see that being the case.

No, I said we don't do it currently. Just asking your opinion on if you think it's important.

As far as exists, I think that's creating more complication than is necessary. We should either do it or not. Supporting both will lead to maintenance/documentation/support headaches.

[stephenplusplus]

To be less harsh on exists, it's a perfectly valid suggestion, so we can still see what others think.

And just another note on it, since I just noticed this, storage.bucket('stephen-notes').create() is pretty confusing to me. I think we should have a top level method on storage to create a bucket, as opposed to this, where we think we get a bucket, but then have to call create.

[ryanseys]

Now that I think about it, I really prefer using storage.bucket(name) as an encapsulation object that we perform methods on. So we should provide storage.bucket(name).exists(callback) but we just assume the developer will use it to check if they are unsure. If a developer KNOWS the bucket exists, we shouldn't force them to use an API call to check. In this case, we also have very minimal overhead in creating a bucket object too. 👍

[stephenplusplus]

I really prefer using storage.bucket(name) as an encapsulation object that we perform methods on.

Yes.

If a developer KNOWS the bucket exists, we shouldn't force them to use an API call to check.

That does make sense. I would vote to remove the exists method, and it they don't know it exists, they should use a bucket-listing method to figure it out from the results there.

Can you now tackle how you see working with "buckets" methods, like listing, creating, and deleting?

[ryanseys]

Hmm... storage.createBucket(name, cb) or storage.bucket(name).create(cb)... Yes I can see the advantage to having both.

Let's go back to first year and recall that calling a method on something is delegating it to perform some action. Delegation. So, storage is responsible for creating a bucket. storage.createBucket() makes sense then. And similarly storage.bucket(name).list(cb) makes sense as opposed to storage.listFiles(bucket, cb) because the bucket's job is to list its files / upload files / delete files, not storage's job.

So then we are left with storage.deleteBucket(name, cb), and storage.listBuckets(cb) as opposed to storage.bucket(name).delete(cb) and... well storage.bucket().list() doesn't make any sense.

So we could do:

storage.createBucket(name);

// or

var bucket = storage.bucket('name');
storage.create(bucket, cb);

So now my question is should we do:

var bucket = storage.bucket('name');
bucket.upload('filename', cb);

// or

// blah, I don't like this one.
storage.upload('bucketName', 'filename', cb)

// or

var bucket = storage.bucket('name');
var file = bucket.file('filename');
bucket.upload(file, cb);

[ryanseys]

(I'm really indecisive). Lemme sit on this and come up with something more concrete taking into consideration the different layers of storage.

[stephenplusplus]

No problem.

var bucket = storage.bucket('name');
bucket.upload('filename', cb);

I think operations to a bucket should happen on the storage level, and operations on a bucket should happen on the bucket level, basically matching how the API is broken apart: https://developers.google.com/storage/docs/json_api/v1. The storage module, to me, needs to look like this:

storage = {
  bucket: function() {},
  createBucket: function(name, callback) {
    apiCreateBucket(name, function() {
      if (err) { callback(err); return; }
      // return a bucket instance to the createBucket callback
      callback(null, storage.bucket(name));
    });
  },
  deleteBucket: function(callback) {},
  // also could be listBuckets, can't remember which we've used in the past
  getBuckets: function(callback) {}
};

storage.bucket should be kept as a getter of an existing bucket, and not confuse the hierarchy with:

storage.bucket('new-bucket').create();

I know the one liner is nice, but it's backwards. "Get -> Create", should be "Create -> Get".

storage.bucket('my-bucket').delete(function(err) {});

would be confused with

storage.bucket('my-bucket').remove('', function(err) {});

The first deleting a bucket from the bucket instance, and the second deleting a file in the bucket. Better to keep bucket removal clear, and on the storage level.

Those are just my thoughts!

[stephenplusplus]

One last concept for your review, which is going back to my first example from an earlier comment:

var storage = gcloud.storage;

var buckets = new storage.Buckets(/*credentials*/);
// buckets instanceof Buckets

buckets.list(function(err, buckets) {
  // buckets = Bucket[]
});

buckets.get('Photos', function(err, photos) {
  // photos instanceof Bucket
});

buckets.create('Albums', function(err, albums) {
  // albums instanceof Bucket
});

I liked this because of the hierarchical logic. You create/add a bucket to a collection of buckets, and you get a bucket from a collection of buckets.

Using the advancements we've made in the rest of the thread, with some small tweaks, this doesn't look too bad to me, and maintains that hierarchy.

var buckets = gcloud.storage.buckets;

buckets.list(function(err, results) {
  // results = Bucket[]
});

var PhotosBucket = buckets.get('Photos');
// PhotosBucket instanceof Bucket

buckets.create('Albums', function(err, albums) {
  // albums instanceof Bucket

  // New delete method example:
  buckets.delete('Albums', function(err) {});
});

So, using your earlier use case of:

var bucket = storage.bucket('name');
bucket.upload('filename', cb);

This would look like:

var bucket = storage.buckets.get('name');
bucket.upload('filename', cb);

Which is a loss in terms of character count, but I think a plus in terms of understandability when you consider we need to share the storage api with other methods. In other words, if our api was only about getting a bucket, storage.bucket() makes the most sense. But, since we have to support these other things, like creating, deleting, and listing buckets (plus the operations to a bucket), the brevity doesn't really lend itself to clarity. Just my opinion, hope some sense could be drawn from this.

[stephenplusplus]

Refactored discussion: #194 :)