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

storage: API proposal #194

Closed
ryanseys opened this issue Sep 7, 2014 · 23 comments
Closed

storage: API proposal #194

ryanseys opened this issue Sep 7, 2014 · 23 comments
Assignees
Labels
api: storage Issues related to the Cloud Storage API. 🚨 This issue needs some love. triage me I really want to be triaged.

Comments

@ryanseys
Copy link
Contributor

ryanseys commented Sep 7, 2014

New

http://stephenplusplus.github.io/gcloud-node/#/docs/master/storage

Old

Moved to docs: https://docs.google.com/document/d/1SG7DYKEjzX8MiYcPtrKunprT6fgLXIFDsdoVA8ETdow

I took some time to go through what calls the current storage API supports and tried to redesign them to be easier to understand and encapsulate what most developers would like to do with the api. This covers buckets, files, and ACL on them both. The API I propose is below. Let me know if I missed anything or you have questions or a better idea 👍

Initialization

var gcloud = require('gcloud')({ /* optional stuff like credentials */ });
var storage = gcloud.storage;

Buckets

Reference to a bucket (local only, no network request)

var myBucket = storage.bucket('ryan-files');

Create a bucket

storage.createBucket('ryan-files', cb);

Delete a bucket

storage.deleteBucket('ryan-files', cb);

List all buckets

storage.listBuckets(cb);

Get metadata about a bucket

storage.getBucket('bucket-name', cb);
// or (pick one)
storage.bucket('ryan-files').getInfo(cb);

Files/Objects

Reference to a file/object in a bucket

myBucket.file('stephen.png');
// or (pick one)
myBucket.object('stephen.png');

Upload an object to a bucket

myBucket.upload('local_filename.png', { name: 'remote_filename.png' /*, other opts too */ }, cb);
// or more simply
myBucket.upload('local_filename.png', 'remote_filename.png', cb);

Remove an object from a bucket

myBucket.remove('remote_filename.png', cb);
// or (pick one)
myBucket.delete('remote_filename.png', cb);

Copy an object from one bucket to another

myBucket.copy('ryan.png', storage.bucket('stephen-files').file('stephen.png'), cb);

ACLs

Create an ACL?

List of scope/permission pairs (may be overkill?)

var myACL = storage.acl(['entity', 'permission', 'entity2', 'permission2']);

Set default object ACL for all objects added to bucket

myBucket.setDefaultACL(myACL, cb);

Clear default ACL for all objects added to bucket

myBucket.clearDefaultACL(myACL, cb);

Get default ACL for all objects added to bucket

myBucket.getDefaultACL(myACL, cb);

Set ACL of bucket

myBucket.setACL(myACL, cb);

Set ACL of file/object

myBucket.file('myfile.png').setACL(myACL, cb);

Clear custom ACL for file/object

myBucket.file('myfile.png').clearACL(cb);

Clear custom ACL for bucket

myBucket.clearACL(cb);

Get ACL of a file/object

myBucket.file('myfile.png').getACL(cb);

Get ACL of a bucket

myBucket.getACL(cb);

Channels

Stop watching resources through this channel

This is provided by storage.channels.stop() in google-api-nodejs-client

storage.stopChannels();
@stephenplusplus
Copy link
Contributor

I like this proposal 💯

Get metadata about a bucket

storage.getBucket('bucket-name', cb);
// or (pick one)
storage.bucket('ryan-files').getInfo(cb);

storage.bucket()

Though I prefer getMetadata.

Reference to a file/object in a bucket

myBucket.file('stephen.png');
// or (pick one)
myBucket.object('stephen.png');

Thanks for the shout out! :)

I really like the file object. It allows us another child in the hierarchy, allowing logical separation of actions. I think they should be full featured objects, removing a lot of functionality from bucket. Consider if all file objects are duplex streams:

var ryan = myBucket.file('ryan.gif');

// upload a file to your bucket's ryan.gif file
fs.createReadStream('local_file.png').pipe(ryan);

// pipe a readable stream of your bucket's ryan.gif file to a destination
ryan.pipe(fs.createWriteStream('local_ryan.gif'));

// copy to another bucket - replace myBucket.copy
ryan.pipe(anotherBucket.file('ryan-clone.gif'));

// get file metadata - replace myBucket.stat
ryan.stat(cb);

// set file metadata - replace myBucket.write's metadata write only functionality
ryan.setMetadata({});

// delete file - replace myBucket.delete
ryan.delete(cb); // (sorry)

Footnote: let's do a little better housekeeping. This is the result of a lengthy discussion at #168 based around supporting the creation of buckets. I agree with flushing that convo and starting anew, but we should make sure these discussions are linked, and old ones are closed.

@ryanseys
Copy link
Contributor Author

ryanseys commented Sep 8, 2014

If ryan.delete(cb) deletes the file then why doesn't myBucket.delete(cb) delete the bucket? (we had this whole delegation conversation earlier).

If we provide ryan.pipe(anotherBucket.file('ryan-clone.gif')); then there's no reason why we shouldn't or couldn't also provide myBucket.copy() (for developers who don't want to use the streaming interface and just want plain methods).

@stephenplusplus
Copy link
Contributor

If ryan.delete(cb) deletes the file then why doesn't myBucket.delete(cb) delete the bucket? (we had this whole delegation conversation earlier).

Fair. But, the other part of my point in regards to myBucket.delete was it would be confused with remove, which removes a file. If we have a file object to handle file things, I have less of an issue with myBucket.delete and myFile.delete. Did you agree with me from the other conversation? How do you look at it/which do you prefer?

Sure, we can provide quick methods as well, and we can internally use the stream api:

myBucket.copy('local_file.png', anotherBucket.file('ryan-clone.gif'));
// where...
Bucket.prototype.copy = function(localFilepath, destinationFileObject, callback) {
  fs.createReadStream(localFilepath).pipe(destinationFileObject)
    .on('error', callback)
    .on('complete', callback);
};

@ryanseys
Copy link
Contributor Author

ryanseys commented Sep 8, 2014

Yes I like myBucket.delete(cb) delete a bucket and myBucket.getMetadata(cb) get bucket metadata etc. The one thing you didn't like from the previous discussion was myBucket.create(cb) and storage.listBuckets() would still have to remain.

@stephenplusplus
Copy link
Contributor

The one thing you didn't like from the previous discussion was myBucket.create(cb) and storage.listBuckets() would still have to remain.

I may not understand you, but isn't it now storage.createBucket? I like creating a bucket from the storage object, not from an already created Bucket instance. When I have a Bucket, I think "this refers to an existing remote endpoint," so creating one from that object breaks that model. That's not the same for deleting, as it makes sense to delete an already existing bucket from a Bucket instance.

And storage.listBuckets is cool with me. I think I suggested getBuckets or listBuckets, and I'm still +1 to either.

@ryanseys
Copy link
Contributor Author

ryanseys commented Sep 8, 2014

Yeah, this makes sense. storage.createBucket() is better than myBucket.create() and either listBuckets or getBuckets, I'm indifferent.

@silvolu
Copy link
Contributor

silvolu commented Sep 8, 2014

Thanks guys, this is a great discussion!
Could we move it to a public Google Doc? That would allow us to keep an updated snapshot of the proposed API at the top of the doc and a track of considered alternatives, and I think it would make it clearer for newcomers to follow the discussion and understand where are we at.

@rakyll
Copy link
Contributor

rakyll commented Sep 9, 2014

Could we move this to Docs? I have comments,

@stephenplusplus
Copy link
Contributor

@stephenplusplus
Copy link
Contributor

@silvolu - should I jump on putting this into code next?

@silvolu
Copy link
Contributor

silvolu commented Sep 30, 2014

I cannot comment ON the doc :/
@rakyll did you manage to add your comments on this proposal?

@ryanseys
Copy link
Contributor Author

@silvolu added you to the doc

@stephenplusplus
Copy link
Contributor

@ryanseys
Copy link
Contributor Author

ryanseys commented Oct 7, 2014

I don't think it's recommended convention to name your buckets with a
capital and especially not with the word Bucket in the name. It's
redundant. We should keep with a default project name styled naming
convention, using all lower case words and numbers separated by dashes.
Developers will most likely adopt our conventions if they are clear and
consistent enough for them to follow.

string and options.name are backward in the bucket function docs table :)

@stephenplusplus
Copy link
Contributor

makes-sense. Updated :)

@stephenplusplus stephenplusplus changed the title storage: API rethink proposal storage: API proposal Oct 7, 2014
@ryanseys
Copy link
Contributor Author

ryanseys commented Oct 8, 2014

string and options.name still backward :)

screen shot 2014-10-07 at 9 50 27 pm

@ryanseys
Copy link
Contributor Author

ryanseys commented Oct 8, 2014

stroage.getBuckets({ should be storage.getBuckets({

@ryanseys
Copy link
Contributor Author

ryanseys commented Oct 8, 2014

Initial example of uploading and downloading a file should be more prominent. Also, can we include that example in the bucket.upload example as an alternative.

Also, if we have an upload function, can we also have a download function? Or is that kinda silly?

Edit: Be more prominent by perhaps updating the header from "Example" to something more descriptive like "Example uploading and downloading a file"

@stephenplusplus
Copy link
Contributor

Initial example of uploading and downloading a file should be more prominent.

Which example?

string and options.name still backward :)

What should it look like?

@ryanseys
Copy link
Contributor Author

ryanseys commented Oct 8, 2014

The main example in the File description.

They should swap cells. Should be options.name then string. See the row above it, it has options then string,object

sofisl pushed a commit that referenced this issue Nov 11, 2022
* chore(main): release 2.2.1

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
sofisl pushed a commit that referenced this issue Nov 11, 2022
- [ ] Regenerate this pull request now.

PiperOrigin-RevId: 468735472

Source-Link: googleapis/googleapis@cfa1b37

Source-Link: googleapis/googleapis-gen@09b7666
Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiMDliNzY2NjY1NjUxMGY1YjAwYjg5M2YwMDNhMGJhNTc2NmY5ZTI1MCJ9
sofisl pushed a commit that referenced this issue Nov 11, 2022
* chore(main): release 2.1.1

* 🦉 Updates from OwlBot post-processor

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
sofisl pushed a commit that referenced this issue Nov 16, 2022
[![Mend Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [linkinator](https://github.com/JustinBeckwith/linkinator) | [`^2.7.0` -> `^4.0.0`](https://renovatebot.com/diffs/npm/linkinator/2.16.2/4.0.2) | [![age](https://badges.renovateapi.com/packages/npm/linkinator/4.0.2/age-slim)](https://docs.renovatebot.com/merge-confidence/) | [![adoption](https://badges.renovateapi.com/packages/npm/linkinator/4.0.2/adoption-slim)](https://docs.renovatebot.com/merge-confidence/) | [![passing](https://badges.renovateapi.com/packages/npm/linkinator/4.0.2/compatibility-slim/2.16.2)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://badges.renovateapi.com/packages/npm/linkinator/4.0.2/confidence-slim/2.16.2)](https://docs.renovatebot.com/merge-confidence/) |

---

### Release Notes

<details>
<summary>JustinBeckwith/linkinator</summary>

### [`v4.0.2`](https://github.com/JustinBeckwith/linkinator/releases/tag/v4.0.2)

[Compare Source](https://github.com/JustinBeckwith/linkinator/compare/v4.0.1...v4.0.2)

##### Bug Fixes

-   address srcset parsing with multiple spaces ([#&#8203;512](https://github.com/JustinBeckwith/linkinator/issues/512)) ([fefb5b6](https://github.com/JustinBeckwith/linkinator/commit/fefb5b6734fc4ab335793358c5f491338ecbeb90))

### [`v4.0.1`](https://github.com/JustinBeckwith/linkinator/releases/tag/v4.0.1)

[Compare Source](https://github.com/JustinBeckwith/linkinator/compare/v4.0.0...v4.0.1)

##### Bug Fixes

-   properly parse srcset attribute ([#&#8203;510](https://github.com/JustinBeckwith/linkinator/issues/510)) ([9a8a83c](https://github.com/JustinBeckwith/linkinator/commit/9a8a83c35182b3cd4daee62a00f156767fe5c6a7))

### [`v4.0.0`](https://github.com/JustinBeckwith/linkinator/releases/tag/v4.0.0)

[Compare Source](https://github.com/JustinBeckwith/linkinator/compare/v3.1.0...v4.0.0)

##### Features

-   create new release with notes ([#&#8203;508](https://github.com/JustinBeckwith/linkinator/issues/508)) ([2cab633](https://github.com/JustinBeckwith/linkinator/commit/2cab633c9659eb10794a4bac06f8b0acdc3e2c0c))

##### BREAKING CHANGES

-   The commits in [#&#8203;507](https://github.com/JustinBeckwith/linkinator/issues/507) and [#&#8203;506](https://github.com/JustinBeckwith/linkinator/issues/506) both had breaking changes.  They included dropping support for Node.js 12.x and updating the CSV export to be streaming, and to use a new way of writing the CSV file.  This is an empty to commit using the `BREAKING CHANGE` format in the commit message to ensure a release is triggered.

### [`v3.1.0`](https://github.com/JustinBeckwith/linkinator/releases/tag/v3.1.0)

[Compare Source](https://github.com/JustinBeckwith/linkinator/compare/v3.0.6...v3.1.0)

##### Features

-   allow --skip to be defined multiple times ([#&#8203;399](https://github.com/JustinBeckwith/linkinator/issues/399)) ([5ca5a46](https://github.com/JustinBeckwith/linkinator/commit/5ca5a461508e688de12e5ae6b4cfb6565f832ebf))

### [`v3.0.6`](https://github.com/JustinBeckwith/linkinator/releases/tag/v3.0.6)

[Compare Source](https://github.com/JustinBeckwith/linkinator/compare/v3.0.5...v3.0.6)

##### Bug Fixes

-   **deps:** upgrade node-glob to v8 ([#&#8203;397](https://github.com/JustinBeckwith/linkinator/issues/397)) ([d334dc6](https://github.com/JustinBeckwith/linkinator/commit/d334dc6734cd7c2b73d7ed3dea0550a6c3072ad5))

### [`v3.0.5`](https://github.com/JustinBeckwith/linkinator/releases/tag/v3.0.5)

[Compare Source](https://github.com/JustinBeckwith/linkinator/compare/v3.0.4...v3.0.5)

##### Bug Fixes

-   **deps:** upgrade to htmlparser2 v8.0.1 ([#&#8203;396](https://github.com/JustinBeckwith/linkinator/issues/396)) ([ba3b9a8](https://github.com/JustinBeckwith/linkinator/commit/ba3b9a8a9b19d39af6ed91790135e833b80c1eb6))

### [`v3.0.4`](https://github.com/JustinBeckwith/linkinator/releases/tag/v3.0.4)

[Compare Source](https://github.com/JustinBeckwith/linkinator/compare/v3.0.3...v3.0.4)

##### Bug Fixes

-   **deps:** update dependency gaxios to v5 ([#&#8203;391](https://github.com/JustinBeckwith/linkinator/issues/391)) ([48af50e](https://github.com/JustinBeckwith/linkinator/commit/48af50e787731204aeb7eff41325c62291311e45))

### [`v3.0.3`](https://github.com/JustinBeckwith/linkinator/releases/tag/v3.0.3)

[Compare Source](https://github.com/JustinBeckwith/linkinator/compare/v3.0.2...v3.0.3)

##### Bug Fixes

-   export getConfig from index ([#&#8203;371](https://github.com/JustinBeckwith/linkinator/issues/371)) ([0bc0355](https://github.com/JustinBeckwith/linkinator/commit/0bc0355c7e2ea457f247e6b52d1577b8c4ecb3a1))

### [`v3.0.2`](https://github.com/JustinBeckwith/linkinator/releases/tag/v3.0.2)

[Compare Source](https://github.com/JustinBeckwith/linkinator/compare/v3.0.1...v3.0.2)

##### Bug Fixes

-   allow server root with trailing slash ([#&#8203;370](https://github.com/JustinBeckwith/linkinator/issues/370)) ([8adf6b0](https://github.com/JustinBeckwith/linkinator/commit/8adf6b025fda250e38461f1cdad40fe08c3b3b7c))

### [`v3.0.1`](https://github.com/JustinBeckwith/linkinator/releases/tag/v3.0.1)

[Compare Source](https://github.com/JustinBeckwith/linkinator/compare/v3.0.0...v3.0.1)

##### Bug Fixes

-   decode path parts in local web server ([#&#8203;369](https://github.com/JustinBeckwith/linkinator/issues/369)) ([4696a0c](https://github.com/JustinBeckwith/linkinator/commit/4696a0c38c341b178ed815f47371fca955979feb))

### [`v3.0.0`](https://github.com/JustinBeckwith/linkinator/releases/tag/v3.0.0)

[Compare Source](https://github.com/JustinBeckwith/linkinator/compare/v2.16.2...v3.0.0)

##### Bug Fixes

-   **deps:** update dependency chalk to v5 ([#&#8203;362](https://github.com/JustinBeckwith/linkinator/issues/362)) ([4b17a8d](https://github.com/JustinBeckwith/linkinator/commit/4b17a8d87b649eaf813428f8ee6955e1d21dae4f))

-   feat!: convert to es modules, drop node 10 ([#&#8203;359](https://github.com/JustinBeckwith/linkinator/issues/359)) ([efee299](https://github.com/JustinBeckwith/linkinator/commit/efee299ab8a805accef751eecf8538915a4e7783)), closes [#&#8203;359](https://github.com/JustinBeckwith/linkinator/issues/359)

##### BREAKING CHANGES

-   this module now requires node.js 12 and above, and has moved to es modules by default.

</details>

---

### Configuration

📅 **Schedule**: Branch creation - "after 9am and before 3pm" (UTC), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, click this checkbox.

---

This PR has been generated by [Mend Renovate](https://www.mend.io/free-developer-tools/renovate/). View repository job log [here](https://app.renovatebot.com/dashboard#github/googleapis/nodejs-retail).
sofisl pushed a commit that referenced this issue Nov 30, 2022
Source-Author: F. Hinkelmann <franziska.hinkelmann@gmail.com>
Source-Date: Thu Jul 23 01:45:04 2020 -0400
Source-Repo: googleapis/synthtool
Source-Sha: 3a00b7fea8c4c83eaff8eb207f530a2e3e8e1de3
Source-Link: googleapis/synthtool@3a00b7f
sofisl pushed a commit that referenced this issue Jan 10, 2023
sofisl pushed a commit that referenced this issue Jan 17, 2023
sofisl pushed a commit that referenced this issue Jan 26, 2023
Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: storage Issues related to the Cloud Storage API. 🚨 This issue needs some love. triage me I really want to be triaged.
Projects
None yet
Development

No branches or pull requests

6 participants