complete Google Cloud storage implementation (and tests)

[aozarov]

No description provided.

[aozarov]

@mziccard assigning this one to you, if you don't mind.

Lets make sure we get a good coverage for both API and SPI layer.
Do you mind to explore the possibility of a fake/local-service (similar to what we have for the Datastore)?

[mziccard]

I had a look around and there seem to be no fake/local-service for Storage as the one we use for the Datastore. In the appengine-gcs-client they save files locally when the application in run in the development server. But they do that at the library level, so no emulator is used.

In gcloud-node they use the actual Cloud Storage to run system tests. They require two environment variables to be set (GCLOUD_TESTS_PROJECT_ID and GCLOUD_TESTS_KEY) and run tests against that project.

[aozarov]

Using production GCS is probably OK for integration tests that we can make Travis run (until gcloud emulators provides support for GCS.
Are you OK with taking this task - adding an integration test[s] and completing the unit-tests (with mocks when necessary)?

[mziccard]

Yes sure I already started :)

[aozarov]

@mziccard as part of this issue effort can you also go over the storage related open issues and lets
try to resolve some of them?

Issue #109 - My preference, which I feel consistent with the spirit of the JDK and guava, is that anything that can be serializable (especially plain data models/containers) should be as such, but when not a of plain data models (such as BlobInfo, BucketInfo,...) I don't feel strongly about it.

Issue #107 - My preference to wait for Java 8, but I don't mind to change the signUrl signature to accept
also a TimeUnit if you think that would be better that just showing it in the javadoc as we do now.

Issue #62 - I am not in the camp of providing a "key" like reference to Blobs, as I didn't see a true need for this in this API (as oppose to Datastore which we have one) but I don't feel as strongly about it as I feel against using a true URI (as originally proposed). In any case, because this may significantly effect
the API is labeled as "Needed for public announcement".

Issue #60 - This is also a big one and therefore marked as "Needed for public announcement". We need
to see if the bug in the apiary client was fixed and even with or without the fix if we really want the update
to use patch and make sure documentation is clear.

Also: There was a proposal to change signUrl to return a URI instead of a URL but I didn't see a dedicated issue for it. and some of the //todo comments in the code could be removed now (e.g. in StorageImpl "todo: configure timeouts"...

[mziccard]

Sure! I started going through the issues and //todos,

On the signURL/signURI matter it was discussed in this PR #93. Seems that using URI was preferred but I agree with you that this functionality is know everywhere (docs, stackoverflow, Amazon S3 docs and API) as getting a signed URL. BTW, Amazon S3 uses URL:

URL s = s3client.generatePresignedUrl(generatePresignedUrlRequest);

[aozarov]

I am fine with keeping it as is then.

[aozarov]

@mziccard so what else is pending before we can close this issue?

I consider these as dependencies:
#106 - I think we decided not to do anything about it?
#291 - Is about to be resolved.
#306
#193 - I actually don't see that as a true blocker for this issue, however I think it would be very nice
(maybe we just need to add exists to the RPC layer?)

Anything else?

[mziccard]

I think blockers are only:
#106 I would do nothing as I am not convinced about the alternatives
#291 We are about to close it with #294
#306 As soon as #294 gets merged I'll send a PR for this

I don't think #193 should be a blocker. I would add support for selected fields to all gets (through vararg of field enums?) rather than adding exists to the RPC layers (it seems to me as a more complete solution)

[aozarov]

Great, so only #306 is a blocker for this issue and your approach for fixing the non-blocker #193 issue sounds good to me.

[aozarov]

Closing this issue as all associated issues are closed.