Adds adapter for Google Cloud Client Library #427

PanzerLlama · 2016-09-09T12:04:39Z

Hi,

Adapter for http://googlecloudplatform.github.io/google-cloud-php/

This change is

…rary 0.8.0) - general cleanup - added extra unit tests

akerouanton · 2017-03-14T11:39:58Z

Hello @PanzerLlama,

What's the difference with the current GoogleCloudStorage?

PanzerLlama · 2017-03-14T12:04:27Z

Hi @NiR- ,

Google Cloud Client Library is just an alternative to GCS PHP API lib. Quote from the homepage (https://googlecloudplatform.github.io/google-cloud-php/#/):

"What is the relationship between the Google Cloud Client Library and the Google APIs PHP Client?

[...] The Google Cloud Client Library is built specifically for the Google Cloud Platform and is the recommended way to integrate Google Cloud APIs into your PHP applications. If your application requires both Google Cloud Platform and other Google APIs, the 2 libraries may be used by your application."

Personally I found the documentation for GCCL to be clearer and easier to work with, also this lib is actively developed by Google programmers contrary to the GCS which is generated automatically. In a nutshell - Google recommends GCCL if the app is developed in single language (PHP).

More details on the differences can be found here: https://cloud.google.com/storage/docs/json_api/v1/libraries

akerouanton · 2017-03-28T21:41:54Z

@PanzerLlama Could you update the existing adapter rather than creating a new one?

PanzerLlama · 2017-03-30T12:35:41Z

@NiR- - not sure if this works out well since these are 2 different libraries sitting on top of the Google Cloud API so adding support to the existing adapter would generate a lot of extra code in it. Also the constructor of the existing adapter requires \Google_Service_Storage and my adapter uses \Google\Cloud\Storage\StorageClient so looks like merging will create BC break. (?) I think since these are 2 different libraries it feels right to have 2 different adapters.

edit sometime later: ok, the BC break note is not true. I think I don't get the pros of merging these 2 into one adapter but if you think it's vital I will come into line :) (won't be that quick though).

AntoineLelaisant · 2017-03-30T15:58:00Z

composer.json

@@ -38,7 +38,8 @@
        "phpunit/phpunit": "3.7.*",
        "microsoft/windowsazure": "dev-master",
        "mikey179/vfsStream": "~1.2.0",
-        "league/flysystem": "~1.0"
+        "league/flysystem": "~1.0",
+        "google/cloud": ">=0.20.1"


This dep should be added in the suggest section, because there is no need to install this library if you don't need to use your GoogleCloud adapter.

We opened an RFC to find a better solution to this problematic #466

One more point: shouldn't we require 'google/cloud-storage' instead of the entire 'google/cloud' library (as it's recommended here https://github.com/GoogleCloudPlatform/google-cloud-php/tree/master/src/Storage in the README.md section) ?

Agreed & fixed.

AntoineLelaisant · 2017-04-13T09:12:51Z

Hi @PanzerLlama Sorry for not responding for a long time but we currently having a big internal discussion about the future of Gaufrette (see #466 and #464). The main idea for now is to split adapters into meta-packages and limit the amount of adapters we have to maintain.

At the moment we already have a GoogleCloudStorage adapter based on https://github.com/google/google-api-php-client and we don't want to maintain two adapters for the same functionality at the end. Moreover we could not simply replace the GoogleCloudStorage adapter by yours because it would bring BC breaks.

By the way, your implementation seems cool and it would be a waste to simply throw it away.

So I see two solutions:

Let this PR pending until the release of Gaufrette v1.0
-> Deprecate the actual GoogleCloudStorage in Gaufrette v0.4
-> Replace it by your adapter in v1.0
Create a separated repo for your adapter (so not maintained by KnpLabs)
We would be happy to mention it in the Gaufrette's README.md

akerouanton · 2017-04-17T20:45:23Z

@PanzerLlama Indeed, seems to be a bad idea to reuse the current Adapter.

@AntoineLelaisant I would go for the first option, but without even waiting the v1.0 release. If we want to deprecate the old adapter in the v0.4 release we need this new adapter to be available.

PanzerLlama · 2017-04-18T07:42:47Z

Hi @AntoineLelaisant - let's do what you think is best. I'll be away now for around a week but will follow the feedback.

akerouanton · 2017-05-20T02:04:42Z

src/Gaufrette/Adapter/GoogleCloudClientStorage.php

+    {
+        $this->isBucket();
+        $object = $this->bucket->object($this->computePath($key));
+        $object->delete();


Same question as previous comment.

akerouanton · 2017-05-20T02:06:09Z

src/Gaufrette/Adapter/GoogleCloudClientStorage.php

+
+        $object = $this->bucket->object($this->computePath($key));
+
+        $properties = array_replace_recursive(


Why replace_recursive? A simple array_replace should do the work, no?

akerouanton · 2017-05-20T02:08:08Z

src/Gaufrette/Adapter/ResourcesSupporter.php

+ *
+ * @author  Lech Buszczynski <lecho@phatcat.eu>
+ */
+interface ResourcesSupporter


I'm not sure about this one? What's the purpose of such interface?

PanzerLlama · 2017-05-23T07:49:42Z

@NiR- Will do the fixes sometime next week. Thanks for the feedback.

It should be re-added when KnpLabs#427 is merged.

akerouanton · 2017-06-13T21:26:54Z

@PanzerLlama Is it ready to be merged?

PanzerLlama · 2017-06-14T08:17:16Z

@NiR- yes, done all requested changed (may improve docs later but for now I am done)

bwegrzyn · 2017-06-17T21:17:04Z

Can't wait for this adapter to be merged. 👍

Thanks for the hard work.

Matan · 2017-09-27T16:18:07Z

Hi! Any update on this? Would be great if we could get this merged in. 👍

Conflicts: phpunit.xml.dist

PanzerLlama · 2018-05-07T12:50:18Z

@NiR- I am not sure why this still is labelled "needs rework" - I am happy to do changes but as far as I can see I did the updates that were requested.

PanzerLlama · 2018-07-06T11:39:22Z

For anyone who wants to use this adapter (workaround):

copy this file into your adapters directory: https://github.com/PanzerLlama/Gaufrette/blob/master/src/Gaufrette/Adapter/GoogleCloudClientStorage.php
read: https://github.com/PanzerLlama/Gaufrette/blob/master/doc/adapters/google-cloud-client-storage.md

You can also copy the test but its not required. I use this adapter for well over a year w/o issues.

It should be re-added when KnpLabs#427 is merged.

PanzerLlama added 11 commits September 9, 2016 13:50

d9cca50

f6520fe

d7ec673

8060ad6

759b856

- added functional "rename" (now supported by Google Cloud Client Lib…

c0de2c7

…rary 0.8.0) - general cleanup - added extra unit tests

- added functional "rename" (now supported by Google Cloud Client Lib…

d1e31ef

…rary 0.8.0) - general cleanup - added extra unit tests

342b015

- fix for reading metadata from GCP files

924a41f

- fix for reading metadata from GCP files

294abce

Merge origin/master

dcd7f08

PanzerLlama closed this Mar 14, 2017

PanzerLlama reopened this Mar 14, 2017

akerouanton mentioned this pull request Mar 24, 2017

[RFC] Reboot Gaufrette #466

Open

akerouanton added this to the v1.0 milestone Mar 28, 2017

akerouanton added the need rework label Mar 28, 2017

AntoineLelaisant reviewed Mar 30, 2017

View reviewed changes

PanzerLlama added 4 commits March 31, 2017 11:56

Merge origin/master

30fd12e

Merge origin/master

e4bec1b

Merge origin/master

743d1ed

Merge branch 'master' into master

5e20eac

AntoineLelaisant added the Google cloud label Mar 31, 2017

AntoineLelaisant self-assigned this Apr 13, 2017

akerouanton reviewed May 20, 2017

View reviewed changes

PanzerLlama added 4 commits June 2, 2017 13:19

Merged master fixed conflicts

348a3f6

Fixed merges

b72f6b9

Fixed merges

a12adfa

Work in progress for requested changes

58da147

akerouanton pushed a commit to akerouanton/Gaufrette that referenced this pull request Jun 8, 2017

Remove deprecation notice for GoogleCloudStorage

ea302ca

It should be re-added when KnpLabs#427 is merged.

PanzerLlama added 4 commits June 9, 2017 12:42

Requested changes fixes

13cfbca

Requested changes fixes

e5eb7af

Merge branch 'master' into master

f46731f

Fixed composer.json

c465752

PanzerLlama added 2 commits October 26, 2017 11:56

clean-up

f86eb53

Merge remote-tracking branch 'upstream/master'

e4d4825

Conflicts: phpunit.xml.dist

nicolasmure mentioned this pull request Feb 9, 2018

[RFR] Feature/google cloud client adapter #557

Merged

1 task

athos7933 pushed a commit to athos7933/Gaufrette that referenced this pull request Sep 13, 2018

Remove deprecation notice for GoogleCloudStorage

41c7c29

It should be re-added when KnpLabs#427 is merged.

athos7933 pushed a commit to athos7933/Gaufrette that referenced this pull request Sep 13, 2018

Remove deprecation notice for GoogleCloudStorage

f098537

It should be re-added when KnpLabs#427 is merged.

PedroTroller assigned Zusoy Apr 5, 2023

knpEdgar assigned muchafm Jun 6, 2023

PedroTroller assigned FlorianQgb and unassigned AntoineLelaisant May 27, 2024

PedroTroller self-assigned this Oct 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adds adapter for Google Cloud Client Library #427

Adds adapter for Google Cloud Client Library #427

PanzerLlama commented Sep 9, 2016 •

edited by roukmoute

Loading

akerouanton commented Mar 14, 2017

PanzerLlama commented Mar 14, 2017 •

edited

Loading

akerouanton commented Mar 28, 2017

PanzerLlama commented Mar 30, 2017 •

edited

Loading

AntoineLelaisant Mar 30, 2017 •

edited

Loading

PanzerLlama Mar 31, 2017

AntoineLelaisant commented Apr 13, 2017

akerouanton commented Apr 17, 2017

PanzerLlama commented Apr 18, 2017

akerouanton May 20, 2017

akerouanton May 20, 2017

akerouanton May 20, 2017

PanzerLlama commented May 23, 2017

akerouanton commented Jun 13, 2017

PanzerLlama commented Jun 14, 2017

bwegrzyn commented Jun 17, 2017

Matan commented Sep 27, 2017

PanzerLlama commented May 7, 2018

PanzerLlama commented Jul 6, 2018


		$object = $this->bucket->object($this->computePath($key));

		$properties = array_replace_recursive(

Adds adapter for Google Cloud Client Library #427

Are you sure you want to change the base?

Adds adapter for Google Cloud Client Library #427

Conversation

PanzerLlama commented Sep 9, 2016 • edited by roukmoute Loading

akerouanton commented Mar 14, 2017

PanzerLlama commented Mar 14, 2017 • edited Loading

akerouanton commented Mar 28, 2017

PanzerLlama commented Mar 30, 2017 • edited Loading

AntoineLelaisant Mar 30, 2017 • edited Loading

Choose a reason for hiding this comment

PanzerLlama Mar 31, 2017

Choose a reason for hiding this comment

AntoineLelaisant commented Apr 13, 2017

akerouanton commented Apr 17, 2017

PanzerLlama commented Apr 18, 2017

akerouanton May 20, 2017

Choose a reason for hiding this comment

akerouanton May 20, 2017

Choose a reason for hiding this comment

akerouanton May 20, 2017

Choose a reason for hiding this comment

PanzerLlama commented May 23, 2017

akerouanton commented Jun 13, 2017

PanzerLlama commented Jun 14, 2017

bwegrzyn commented Jun 17, 2017

Matan commented Sep 27, 2017

PanzerLlama commented May 7, 2018

PanzerLlama commented Jul 6, 2018

PanzerLlama commented Sep 9, 2016 •

edited by roukmoute

Loading

PanzerLlama commented Mar 14, 2017 •

edited

Loading

PanzerLlama commented Mar 30, 2017 •

edited

Loading

AntoineLelaisant Mar 30, 2017 •

edited

Loading