Have someone from Google review the Cloud Storage implementation

[jgeewax]

Tracking bug for getting a review and feedback from someone on the Cloud Storage team @ Google.

[jboynes]

The semantics of this API look very similar to functionality provided the NIO2 java.nio.file API. Rather than invent an alternative we could use the existing idioms:

StorageService -> FileSystemProvider
Bucket -> FileSystem
StorageObject -> Path
Input/OutputChannel -> FileChannel, SeekableByteChannel et al.
Acl -> FileAttribute, AclEntry

Basic usage example:

URI bucketId = URI.create("gcs://.../my-bucket");
try (FileSystem myBucket = FileSystems.newFileSystem(bucketId, null)) {
    Path someFile = myBucket.getPath("/path/to/someFile.txt");
    Files.copy(someFile, System.out);
}

[Capstan]

That has merit, and some prior art. GCS has some limitations in that there's no real directory structure, just naming conventions. Between that and the eventual consistency of the object index, you end up with unfortunate behavior around trying to delete "directories". It might be a better match to have the StorageService be both the FileSystemProvider and FileSystem, and the Buckets be the root directories, and then disallow directory creation underneath. On the other hand, that tends to break FileSystem consumers' expectations, so we'll be breaking expectations either way. Are FileSystems allowed to operate in a "mode" so the consumer can choose which style they want?

Another issue is that the gcs pseudo-URI has never been documented, and is missing some specific disambiguation about specifying object generations when the object name itself looks like it is ending in a generation, e.g., "objectname#1234". This can likely be addressed, though unclear whether or not in a backwards-compatible manner with ad hoc gcs parser/generators.

[jboynes]

The key would be to make the primary user-facing interface through Path and Files intuitive. There's precedent for directory and root behaviour being a little quirky cf. ZipFS which may or may not have directory nodes or Windows with its drive letters. We would just need to document the quirks.

Different modes can be configured when a FileSystem is created but I think using that to determine how buckets manifest would run into issues with resolving URIs cf. the issues in Windows in URIs for local and UNC paths. Probably doable but possibly confusing e.g. why on Windows does "file:///C:/foo.txt" work but "file://localhost/C:/foo.txt" not.

I actually pulled "gcs:" out of a hat - I had no idea it was a real thing. "gs:" for compatibility with gsutil is another option. IIRC the syntax there is "gs://bucket/path/to/someFile.txt" making bucket the authority and not part of the path. That seems more like a FileSystem identifier than a root directory.

I don't see the ambiguity with generations as I would expect path segment "object#1234" to be encoded to "object%231234" as "1234" is not a fragment identifier. The set of generations for an object could be retrieved using a custom FileAttribute and FileAttributeView.

[aozarov]

The idea of using Java 7 nio.file for accessing Google Cloud Storage was considered but rejected as not all GCS functionality (e.g. composite), behaviour (e.g. delimiter treatment, file names,..) and ACLs could be fully expressed or mapped. The goal for gcloud-java is to express (or to be able to express) all functionality of the service in an easier to use and idiomatic way and to allow possible higher level libraries (such as java 7 FileSystem) to use it and pick and choose what to expose. In fact, we already have such an effort in place and we are in the process of migrating it to use glcoud-java (now it is based on appengine_gcs_client) and finding it a new home (we were thinking of gcloud-java-contrib but this is still open).

[jgeewax]

Just wanted to jump in and clarify the ultimate goal of gcloud-java (and all the gcloud-* libraries in general):

If you are a developer, the gcloud- library should make it easy to do the common tasks for all of our cloud services (for storage, this would be things like ... save a blob, download a blob, generate a signed URL for a blob, etc). It should not provide extremely high-level complex functionality (ie, gcloud-* for Datastore should not provide a full ORM), but the scope is still quite broad.

Using Storage as a reference point here:

We should be able to get a "read stream" on a blob (here is where NodeJS does this: https://github.com/GoogleCloudPlatform/gcloud-node/blob/master/lib/storage/file.js#L390, in S3 in Java we use an InputStream http://docs.aws.amazon.com/AmazonS3/latest/dev/RetrievingObjectUsingJava.html), however we might not expose an complex abstraction that makes GCS look and act like a full file system.

It's a tough balance to strike, however when in doubt we should err on the side of a whether a typical user would want to do this to accomplish a task -- using AWS as a reference point (primarily because they've set a precedent on what developers have come to expect from cloud client libraries).

[aozarov]

I agree, and for that reason I didn't based the storage API on nio.file (even though I think it is nice and should be part of a contrib library that we provide). gcloud-java does provide a way to read a stream from blob via reader(bucket, blobName, options...) that returns a standard readable Java Channel (which could be converted to other types of streams via a standard utility).

[aozarov]

@Capstan issue #14 is only blocked on #306 but I think that one should be considered as a bug-fix and therefore I think the API is ready for its final review.

/cc @BrandonY @mfschwartz

[aozarov]

Review was completed.
Here are the issues that were raised:

blockers:
#363 - We should include generation as part of the BlobId
#359 - BlobReadChannel should fail if content comes from different generations
#354 - Replace BucketInfo.StorageClass and BucketInfo.Location classes with plain string
#351 - We should better document the Storage API

non-blockers:
#353 - Add an option for crc32/md5 client side validation for Storage.readAllBytes
#365 - Limit rpc batch delete calls to ~100
#364 - Consider other options for Batch requests

[jgeewax]

Awesome - thanks for the summary @aozarov !

[aozarov]

All blockers were fixed. Though #365 is not a blocker, the GCS team highly recommend that we fix it before announcement. @mziccard do you mind to take it? After that I think we can close this issue.

[mziccard]

@aozarov Sure, I'm on it!

[aozarov]

#365 was fixed. closing this issues, other non blockers can be tracked independently.