Issue 6810062: JavaScript Sample Application for Google Cloud Storage
|
Created:
13 years, 2 months ago by ziyadm Modified:
10 years, 8 months ago CC:
mmiele, nherring Visibility:
Public. |
DescriptionJavaScript Sample Application for Google Cloud Storage
Patch Set 1 : JavaScript Sample Application for Google Cloud Storage #
Total comments: 22
Patch Set 2 : Addressing comments from code review for Patch Set 1 #MessagesTotal messages: 6
PTAL.
Sign in to reply to this message.
Overall LGTM. I found a couple of typos in coments and suggested a small addition to the README instructions. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... File javascript-client-library-example/README.md (right): https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/README.md:74: with their associated values. Going through the process of gettings these values might not be obvious for someone new to the API. It might help speed up the process of getting started if we added a list of example settings here. Here's a first attempt based on my experience of getting the sample working. var DEFAULT_BUCKET = 'code-sample-bucket'; //This should be a new bucket name to be created. var DEFAULT_OBJECT = 'test.htm; //This should be new object name to be created. var DEFAULT_GROUP = ''group-0000000000000000000000000000000000000000000000000000000000000000'; //Get this value from the API console. var DEFAULT_CONTENT_TYPE = 'text/html'; //This can be any valid MIME type. See: https://developers.google.com/storage/docs/reference-headers#contenttype var DEFAULT_DATA = 'dGVzdGluZyAxMjM='; //URL-safe Base64-encoded data. See: https://developers.google.com/storage/docs/json_api/v1/objects/insert var DEFAULT_ENTITY = 'allUsers'; //Valid values are user-userId, user-email, group-groupId, group-email, allUsers, allAuthenticatedUsers var DEFAULT_ROLE = 'READER''; //Valid values are READER, OWNER var DEFAULT_ROLE_OBJECT = 'READER''; //Valid values are READER, OWNER https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... File javascript-client-library-example/index.html (right): https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:267: * Google Cloud Storage API request to retrieve the list of buckets in "...list of objects..." https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:365: * Google Cloud Storage API request to insert a bucket into "...insert an object.."
Sign in to reply to this message.
This is great! A couple nits and I'd prefer we say insert and get object doesn't work until media transfers work in the javascript client. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... File javascript-client-library-example/index.html (right): https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:60: var DEFAULT_PROJECT = projectId; Can we remove the DEFAULT_ prefix in these constants? I think that would improve readability. Also we use default inside the api -- default project, default object acl. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:135: function updateApiResultEntry(apiRequestName) { This seems like the least interesting function from a cloud storage perspective. Should we move it to the end? Same for the other methods that have little to do with cloud storage's json api. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:259: function listBuckets() { This is the first interesting function. Does javascript allow us to put this after listApiRequestExplanations? https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:315: function getObject() { Is this a media download? If not then we should also remove this function. I'd prefer we have a "we are working on getting this to work" than "this only works for objects < 64k" or even worse not telling people. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:368: function insertObject() { Does this do a media upload? If not we should remove this function.
Sign in to reply to this message.
https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... File javascript-client-library-example/README.md (right): https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/README.md:43: 1. Download the two files: index.html and README.txt (this file). How do I download these two files? https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/README.md:50: b. Again on the APIs console page, add your JavaScript origin (the root Can I save index.html to my desktop and run it there? If so then what is my origin? Or do I need to upload it to a web server somewhere. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... File javascript-client-library-example/index.html (right): https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:320: executeRequest(request, 'getObject'); Basically this needs to use alt=media and handle the resulting redirect to storage.googleapis.com or else this is just getting the metadata. If that is all that's possible then perhaps we should call this getObjectMetadata? +nathan for any additional comments he wants to add.
Sign in to reply to this message.
https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... File javascript-client-library-example/README.md (right): https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/README.md:43: 1. Download the two files: index.html and README.txt (this file). We are assuming here that this code is made available via cloning some repository (i.e., this code has already been downloaded by the user). On 2012/12/07 23:13:52, fejta wrote: > How do I download these two files? https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/README.md:50: b. Again on the APIs console page, add your JavaScript origin (the root This shouldn't need a webserver. We just need to set up the origin to point to wherever the file is being served from. On 2012/12/07 23:13:52, fejta wrote: > Can I save index.html to my desktop and run it there? If so then what is my > origin? Or do I need to upload it to a web server somewhere. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/README.md:74: with their associated values. Agreed -- I will modify these and re-upload a commit. Thanks! On 2012/12/07 19:38:43, jbsimon wrote: > Going through the process of gettings these values might not be obvious for > someone new to the API. It might help speed up the process of getting started if > we added a list of example settings here. Here's a first attempt based on my > experience of getting the sample working. > > var DEFAULT_BUCKET = 'code-sample-bucket'; //This should be a new bucket name to > be created. > var DEFAULT_OBJECT = 'test.htm; //This should be new object name to be created. > var DEFAULT_GROUP = > ''group-0000000000000000000000000000000000000000000000000000000000000000'; > //Get this value from the API console. > var DEFAULT_CONTENT_TYPE = 'text/html'; //This can be any valid MIME type. See: > https://developers.google.com/storage/docs/reference-headers#contenttype > var DEFAULT_DATA = 'dGVzdGluZyAxMjM='; //URL-safe Base64-encoded data. See: > https://developers.google.com/storage/docs/json_api/v1/objects/insert > var DEFAULT_ENTITY = 'allUsers'; //Valid values are user-userId, user-email, > group-groupId, group-email, allUsers, allAuthenticatedUsers > var DEFAULT_ROLE = 'READER''; //Valid values are READER, OWNER > var DEFAULT_ROLE_OBJECT = 'READER''; //Valid values are READER, OWNER https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... File javascript-client-library-example/index.html (right): https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:60: var DEFAULT_PROJECT = projectId; On 2012/12/07 22:49:47, fejta wrote: > Can we remove the DEFAULT_ prefix in these constants? I think that would improve > readability. > > Also we use default inside the api -- default project, default object acl. Done. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:135: function updateApiResultEntry(apiRequestName) { On 2012/12/07 22:49:47, fejta wrote: > This seems like the least interesting function from a cloud storage perspective. > Should we move it to the end? Same for the other methods that have little to do > with cloud storage's json api. Done. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:259: function listBuckets() { Yes, I can certainly do that. I'll update this with another commit to address these comments. On 2012/12/07 22:49:47, fejta wrote: > This is the first interesting function. Does javascript allow us to put this > after listApiRequestExplanations? https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:267: * Google Cloud Storage API request to retrieve the list of buckets in On 2012/12/07 19:38:43, jbsimon wrote: > "...list of objects..." Done. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:315: function getObject() { Done -- I will remove this. On 2012/12/07 22:49:47, fejta wrote: > Is this a media download? If not then we should also remove this function. I'd > prefer we have a "we are working on getting this to work" than "this only works > for objects < 64k" or even worse not telling people. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:320: executeRequest(request, 'getObject'); Done -- will rename this. On 2012/12/07 23:13:52, fejta wrote: > Basically this needs to use alt=media and handle the resulting redirect to > http://storage.googleapis.com or else this is just getting the metadata. > > If that is all that's possible then perhaps we should call this > getObjectMetadata? > > +nathan for any additional comments he wants to add. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:365: * Google Cloud Storage API request to insert a bucket into On 2012/12/07 19:38:43, jbsimon wrote: > "...insert an object.." Done. https://codereview.appspot.com/6810062/diff/2001/javascript-client-library-ex... javascript-client-library-example/index.html:368: function insertObject() { On 2012/12/07 22:49:47, fejta wrote: > Does this do a media upload? If not we should remove this function. Done.
Sign in to reply to this message.
|
