Update patch set 1

Patch Set 1:

(3 comments)

Regarding complexity: I completely agree - we need to make things as simple as possible. But that's exactly what I'm trying to do. I'm attempting to separate concerns and make sure things have a single responsibility - in other words, I want to remove all possible ambiguity for newcomers with this SDK.

The interface is not a general one - it has a very specific purpose. It is supposed to represent a basic TRANSPORT client interface that all other HTTP clients can use. For that reason I wanted our public API to be consistent with other HTTP libraries and as efficient as possible.

The affiliated work around this change makes internal processes more efficient by clarifying concerns and extracting functionality to dedicated areas - thereby removing unnecessary conflations.

Patch-set: 1

2fa3167d72667bdc10d7964ce43aa29df058d9e1

@@ -17,6 +17,24 @@
       "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
       "unresolved": false
     },
+    {
+      "key": {
+        "uuid": "7adec928_8eb0c744",
+        "filename": "src/OpenStack/Common/Transport/ClientInterface.php",
+        "patchSetId": 1
+      },
+      "lineNbr": 98,
+      "author": {
+        "id": 10517
+      },
+      "writtenOn": "2014-05-13T11:06:40Z",
+      "side": 1,
+      "message": "This is a good point. All of these \"verb\" methods (post, get) are only here to offer convenience, because SDKs heavily really on them for internal functionality. \n\nLooking at this again, I agree that these convenience methods are not *essential* to our API. Perhaps we could remove them from the interface, but still offer them as convenience methods in our abstract client. This means that clients which want them can easily extend AbstractClient and still implement ClientInterface.\n\nWhat do you think?",
+      "parentUuid": "7adec928_9d0f232e",
+      "revId": "2fa3167d72667bdc10d7964ce43aa29df058d9e1",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
     {
       "key": {
         "uuid": "7adec928_5d259ba9",
@@ -40,6 +58,24 @@
       "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
       "unresolved": false
     },
+    {
+      "key": {
+        "uuid": "7adec928_6e81bb49",
+        "filename": "src/OpenStack/Common/Transport/ClientInterface.php",
+        "patchSetId": 1
+      },
+      "lineNbr": 124,
+      "author": {
+        "id": 10517
+      },
+      "writtenOn": "2014-05-13T11:06:40Z",
+      "side": 1,
+      "message": "Another good point. There are a few use-cases, though, that I think might require this method:\n\n1. When a Compute client is constructed, for example, it will have the compute region endpoint as its base URL. The `getBaseUrl` method will be needed internally to construct URLs when executing API commands.\n\n2. There is probably also a use-case for a user wanting to know what base URL (API host) they\u0027re hitting. This would be especially useful when debugging.",
+      "parentUuid": "7adec928_5d259ba9",
+      "revId": "2fa3167d72667bdc10d7964ce43aa29df058d9e1",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
+    },
     {
       "key": {
         "uuid": "7adec928_9d3d63be",
@@ -56,6 +92,24 @@
       "revId": "2fa3167d72667bdc10d7964ce43aa29df058d9e1",
       "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
       "unresolved": false
+    },
+    {
+      "key": {
+        "uuid": "7adec928_4e707fa5",
+        "filename": "src/OpenStack/Common/Transport/Guzzle/GuzzleAdapter.php",
+        "patchSetId": 1
+      },
+      "lineNbr": 34,
+      "author": {
+        "id": 10517
+      },
+      "writtenOn": "2014-05-13T11:06:40Z",
+      "side": 1,
+      "message": "If we look at this class and think about its responsibilities, it\u0027s not actually a client. It doesn\u0027t directly create Request objects, send Response objects, etc. What it\u0027s doing instead is _invoking_ Guzzle\u0027s methods, which then handles all the HTTP logic. \n\nI agree with you, though - the interface does not care about implementation details, and nor should it. We wouldn\u0027t need this adapter if Guzzle\u0027s HTTP client implemented the same interface as our HTTP client. But because they\u0027re incompatible interfaces, we need some way to bridge them.\n\nAll this adapter class does is act as a middleman between OUR interface and Guzzle\u0027s. By connecting the dots and patching up dissimilarities, it makes Guzzle satisfy our interface, and that\u0027s all we care about as consumers. But the fact remains that this class is not actually a HTTP client - it doesn\u0027t handle any HTTP logic internally. It\u0027s just an old-fashioned switchboard operator that connects the calls to the right jack. The operator doesn\u0027t participate with or contribute to the actual conversation. To outsiders, as far as they\u0027re concerned, they\u0027re just interacting with OpenStack. \n\nOne of the most important design principles that we want from this SDK is the ability to inject HTTP clients. What this means is that we cannot be tightly couple to one single implementation - we should not have *one* overall HTTP client that handles different underlying libraries differently. All that we require are classes that implement our interface - and this is what this adapter does: the only responsibility it has is making Guzzle satisfy our interface, nothing more.\n\nIf a user wants to inject their own bespoke HTTP client - that\u0027s fine, they wouldn\u0027t need an adapter. All they\u0027d need to do is make sure their class implements our interface (to satisfy all our typehints). You ONLY need adapters for *existing* classes (in 3rd party codebases) that do not implement our interface - like Symfony, Zend Framework 2, etc.",
+      "parentUuid": "7adec928_9d3d63be",
+      "revId": "2fa3167d72667bdc10d7964ce43aa29df058d9e1",
+      "serverId": "4a232e18-c5a9-48ee-94c0-e04e7cca6543",
+      "unresolved": false
     }
   ]
 }