You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@jclouds.apache.org by Christopher Dancy <no...@github.com> on 2016/02/13 21:14:26 UTC
[jclouds-labs] Initial implementation of Keys API (#234)
Initial implementation surrounding the etcd keys API. This initial commit is purposely leaving out tests so as to give reviewers less to chew on. Not too much special is going on here more just laying the groudwork the API itself and future commits to come.
@nacx you were working with me here before, maybe you could look things over? The idea is to get this base commit, with tests once things look good, added and then send in additional PR's for fleshing out this API.
And my apologies AOT for not finishing this work months ago. I bought a new house along with a new baby so things have been a bit busy around these parts. Looking to get back into the swing of things.
You can view, comment on, or merge this pull request online at:
https://github.com/jclouds/jclouds-labs/pull/234
-- Commit Summary --
* Initial implementation of Keys API
-- File Changes --
M etcd/src/main/java/org/jclouds/etcd/EtcdApi.java (4)
M etcd/src/main/java/org/jclouds/etcd/EtcdApiMetadata.java (2)
A etcd/src/main/java/org/jclouds/etcd/domain/keys/Key.java (41)
A etcd/src/main/java/org/jclouds/etcd/domain/keys/Node.java (53)
M etcd/src/main/java/org/jclouds/etcd/fallbacks/EtcdFallbacks.java (11)
A etcd/src/main/java/org/jclouds/etcd/features/KeysApi.java (60)
M etcd/src/main/java/org/jclouds/etcd/handlers/EtcdErrorHandler.java (13)
-- Patch Links --
https://github.com/jclouds/jclouds-labs/pull/234.patch
https://github.com/jclouds/jclouds-labs/pull/234.diff
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + public abstract int modifiedIndex();
> +
> + public abstract int ttl();
> +
> + @Nullable
> + public abstract String value();
> +
> + Node() {
> + }
> +
> + @SerializedNames({ "createdIndex", "dir", "nodes", "expiration", "key", "modifiedIndex", "ttl", "value" })
> + public static Node create(int createdIndex, boolean dir, List<Node> nodes, String expiration, String key,
> + int modifiedIndex, int ttl, String value) {
> + if (nodes == null)
> + nodes = ImmutableList.of();
Just apply the conditional inline when passing the parameter. We'd better avoid changing the reference for input parameters.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52988397
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Christopher Dancy <no...@github.com>.
> + * limitations under the License.
> + */
> +package org.jclouds.etcd.features;
> +
> +import static org.testng.Assert.assertNotNull;
> +import static org.testng.Assert.assertNull;
> +import static org.testng.Assert.assertTrue;
> +
> +import java.util.UUID;
> +
> +import org.jclouds.etcd.BaseEtcdApiLiveTest;
> +import org.jclouds.etcd.domain.keys.Key;
> +import org.testng.annotations.BeforeClass;
> +import org.testng.annotations.Test;
> +
> +@Test(groups = "live", testName = "KeysApiLiveTest", singleThreaded = true)
No. Just a copy over from another live test. Removing now.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r53006671
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -54,7 +55,17 @@ public void handleError(HttpCommand command, HttpResponse response) {
> exception = new IllegalArgumentException(message);
> break;
> case 404:
> - exception = new ResourceNotFoundException(message);
> + if (command.getCurrentRequest().getMethod().equals("DELETE")) {
> + if (command.getCurrentRequest().getEndpoint().getPath().startsWith("/" + EtcdApiMetadata.API_VERSION + "/keys") &&
> + message.contains("Key not found")) {
> + exception = new ResourceNotFoundException("Could not find key to delete: message=" + message, new HttpResponseException(command, response, message));
> + }
> + } else if (command.getCurrentRequest().getMethod().equals("GET")) {
> + if (command.getCurrentRequest().getEndpoint().getPath().startsWith("/" + EtcdApiMetadata.API_VERSION + "/keys") &&
> + message.contains("Key not found")) {
> + exception = new ResourceNotFoundException("Could not find key: message=" + message, new HttpResponseException(command, response, message));
> + }
This seems inconsistent. the methods are already configured with a fallback that looks for the `Could not find key` string. Why should that be treated in the fallback and this message in the error handler? Are there really two different messages to describe the same error?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52902142
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> A lot of this is boiler plate code so I hope it's easy enough to digest. If not let me know and I can break it up further.
This PR was not that big, so feel free to add the other key methods back.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-184605300
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + public abstract int modifiedIndex();
> +
> + public abstract int ttl();
> +
> + @Nullable
> + public abstract String value();
> +
> + Node() {
> + }
> +
> + @SerializedNames({ "createdIndex", "dir", "nodes", "expiration", "key", "modifiedIndex", "ttl", "value" })
> + public static Node create(int createdIndex, boolean dir, List<Node> nodes, String expiration, String key,
> + int modifiedIndex, int ttl, String value) {
> + return new AutoValue_Node(createdIndex, dir,
> + ImmutableList.copyOf(nodes != null ? nodes : ImmutableList.<Node> of()), expiration, key, modifiedIndex,
Why copying an empty list? Why not just: `nodes != null ? ImmutableList.copyOf(nodes) : ImmutableList.<Node> of()`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r53013341
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Christopher Dancy <no...@github.com>.
> + */
> +@Test(groups = "unit", testName = "KeysApiMockTest")
> +public class KeysApiMockTest extends BaseEtcdMockTest {
> +
> + public void testCreateKey() throws Exception {
> + MockWebServer server = mockEtcdJavaWebServer();
> +
> + server.enqueue(new MockResponse().setBody(payloadFromResource("/keys-create.json")).setResponseCode(201));
> + EtcdApi etcdApi = api(server.getUrl("/"));
> + KeysApi api = etcdApi.keysApi();
> + try {
> + Key createdKey = api.createKey("hello", "world");
> + assertNotNull(createdKey);
> + assertTrue(createdKey.node().key().equals("/hello"));
> + assertTrue(createdKey.node().value().equals("world"));
> + assertSent(server, "PUT", "/" + EtcdApiMetadata.API_VERSION + "/keys/hello");
There are no form parameters being sent it's all on the request path:
https://github.com/coreos/etcd/blob/master/Documentation/api.md#setting-the-value-of-a-key
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r53011255
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import java.util.List;
> +
> +import org.jclouds.javax.annotation.Nullable;
> +import org.jclouds.json.SerializedNames;
> +
> +import com.google.auto.value.AutoValue;
> +
> +@AutoValue
> +public abstract class Node {
> +
> + public abstract int createdIndex();
> +
> + public abstract boolean dir();
> +
> + @Nullable public abstract List<Node> nodes();
Can we make this non-nullable? We should use empty lists instead of null ones where possible.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52901150
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> + public void testCreateKey() {
> + Key createdKey = api().createKey(key, value);
> + assertNotNull(createdKey);
> + assertTrue(createdKey.action().equals("set"));
> + assertTrue(createdKey.node().value().equals(value));
> + }
> +
> + @Test(dependsOnMethods = "testCreateKey")
> + public void testGetKey() {
> + Key getKey = api().getKey(key);
> + assertNotNull(getKey);
> + assertTrue(getKey.action().equals("get"));
> + assertTrue(getKey.node().value().equals(value));
> + }
> +
> + @Test(dependsOnMethods = "testGetKey")
Use `@Test(alwaysRun = true)` method to avoid leaking keys if get test fails.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52988914
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> + key = UUID.randomUUID().toString().replaceAll("-", "");
> + value = UUID.randomUUID().toString().replaceAll("-", "");
> + }
> +
> + @Test
> + public void testCreateKeyWithTTL() {
> + String localKey = UUID.randomUUID().toString().replaceAll("-", "");
> + String localValue = UUID.randomUUID().toString().replaceAll("-", "");
> + Key createdKey = api().createKey(localKey, localValue, 1);
> + assertNotNull(createdKey);
> + assertNotNull(createdKey.node().expiration());
> +
> + try {
> + Thread.sleep(3000);
> + } catch (InterruptedException e) {
> + e.printStackTrace();
Remove this. Let the test propagate the exception and fail.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52988554
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Christopher Dancy <no...@github.com>.
@nacx I've addressed the issues you noted above. I removed some of the functionality, specifically pertaining to the various path params, as they are relevant only to code that not yet been added but will come in later commits. Thought this was the best route instead of trying to explain away what they do and why they are there without any actual code to look at.
I also added the mock and live tests. A lot of this is boiler plate code so I hope it's easy enough to digest. If not let me know and I can break it up further. Once this commit is in, and the keys API foundation is in place, the subsequent commits will be smaller and more to the point.
Thanks AOT!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-184434418
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Christopher Dancy <no...@github.com>.
Very awesome. Thanks @nacx. Now for round 2...
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-185732689
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @Test(dependsOnMethods = "testCreateKey")
> + public void testGetKey() {
> + Key getKey = api().getKey(key);
> + assertNotNull(getKey);
> + assertTrue(getKey.action().equals("get"));
> + assertTrue(getKey.node().value().equals(value));
> + }
> +
> + @Test(dependsOnMethods = "testGetKey")
> + public void testDeleteKey() {
> + Key deletedKey = api().deleteKey(key);
> + assertNotNull(deletedKey);
> + assertTrue(deletedKey.action().equals("delete"));
> + assertTrue(deletedKey.prevNode().value().equals(value));
> + }
Use `@Test(alwaysRun = true)` method to avoid leaking keys if get test fails.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52988912
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +@Test(groups = "unit", testName = "KeysApiMockTest")
> +public class KeysApiMockTest extends BaseEtcdMockTest {
> +
> + public void testCreateKey() throws Exception {
> + MockWebServer server = mockEtcdJavaWebServer();
> +
> + server.enqueue(new MockResponse().setBody(payloadFromResource("/keys-create.json")).setResponseCode(201));
> + EtcdApi etcdApi = api(server.getUrl("/"));
> + KeysApi api = etcdApi.keysApi();
> + try {
> + Key createdKey = api.createKey("hello", "world");
> + assertNotNull(createdKey);
> + assertTrue(createdKey.node().key().equals("/hello"));
> + assertTrue(createdKey.node().value().equals("world"));
> + assertSent(server, "PUT", "/" + EtcdApiMetadata.API_VERSION + "/keys/hello");
And how does the server know that the value has to be "world"? That info is sent somewhere in the request, and we need to validate that.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r53013651
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> + MockWebServer server = mockEtcdJavaWebServer();
> +
> + server.enqueue(new MockResponse().setBody(payloadFromResource("/keys-create.json")).setResponseCode(201));
> + EtcdApi etcdApi = api(server.getUrl("/"));
> + KeysApi api = etcdApi.keysApi();
> + try {
> + Key createdKey = api.createKey("hello", "world");
> + assertNotNull(createdKey);
> + assertTrue(createdKey.node().key().equals("/hello"));
> + assertTrue(createdKey.node().value().equals("world"));
> + assertSent(server, "PUT", "/" + EtcdApiMetadata.API_VERSION + "/keys/hello");
> + } finally {
> + etcdApi.close();
> + server.shutdown();
> + }
> + }
Add the mock test for the create with TTL method.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52989336
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
lgtm now. Thanks @cdancy!
I'm sorry, I don't know a way to configure that. You'll have to play a bit with TestNG and the configurations. If you get stuck then ping me and I'll try to give a hand.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-185430293
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
Thanks @cdancy!
Apart from the inline comments, the code must be formatted according to the [jclouds style guide](https://cwiki.apache.org/confluence/display/JCLOUDS/Coding+Standards).
Keep in mind that we need the tests in order to merge this!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-184222052
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> +
> + @Test(dependsOnMethods = "testGetKey")
> + public void testDeleteKey() {
> + Key deletedKey = api().deleteKey(key);
> + assertNotNull(deletedKey);
> + assertTrue(deletedKey.action().equals("delete"));
> + assertTrue(deletedKey.prevNode().value().equals(value));
> + }
> +
> + @Test(dependsOnMethods = "testDeleteKey")
> + public void testGetNonExistentKey() {
> + Key deletedKey = api().getKey(key);
> + assertNull(deletedKey);
> + }
> +
> + @Test(dependsOnMethods = "testGetNonExistentKey")
Same here. There's no need to depend on any test.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52989015
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> + public void testGetKey() {
> + Key getKey = api().getKey(key);
> + assertNotNull(getKey);
> + assertTrue(getKey.action().equals("get"));
> + assertTrue(getKey.node().value().equals(value));
> + }
> +
> + @Test(dependsOnMethods = "testGetKey")
> + public void testDeleteKey() {
> + Key deletedKey = api().deleteKey(key);
> + assertNotNull(deletedKey);
> + assertTrue(deletedKey.action().equals("delete"));
> + assertTrue(deletedKey.prevNode().value().equals(value));
> + }
> +
> + @Test(dependsOnMethods = "testDeleteKey")
Don't depend on other tests. Just try to get a random value.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52988987
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Christopher Dancy <no...@github.com>.
@nacx squash-and-rebase and we can call it done then?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-185431888
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Christopher Dancy <no...@github.com>.
Thanks @nacx. Going over these now. Yea I wanted to flesh out any issues here before adding tests so as not to make the initial review seem monstrous.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-184226288
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> + @Nullable public abstract String expiration();
> +
> + @Nullable public abstract String key();
> +
> + public abstract int modifiedIndex();
> +
> + @Nullable public abstract String ttl();
> +
> + @Nullable public abstract String value();
> +
> + Node() {
> + }
> +
> + @SerializedNames({"createdIndex", "dir", "nodes", "expiration", "key", "modifiedIndex", "ttl", "value"})
> + public static Node create(int createdIndex, boolean dir, List<Node> nodes, String expiration, String key, int modifiedIndex, String ttl, String value) {
> + return new AutoValue_Node(createdIndex, dir, nodes, expiration, key, modifiedIndex, ttl, value);
Make lists immutable.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52901208
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Christopher Dancy <no...@github.com>.
> + String localValue = UUID.randomUUID().toString().replaceAll("-", "");
> + Key createdKey = api().createKey(localKey, localValue, 1);
> + assertNotNull(createdKey);
> + assertNotNull(createdKey.node().expiration());
> +
> + try {
> + Thread.sleep(3000);
> + } catch (InterruptedException e) {
> + e.printStackTrace();
> + }
> +
> + createdKey = api().getKey(localKey);
> + assertNull(createdKey);
> + }
> +
> + @Test(dependsOnMethods = "testCreateKeyWithTTL")
True enough. Removing now.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r53006711
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> +
> +import org.jclouds.javax.annotation.Nullable;
> +import org.jclouds.rest.annotations.Fallback;
> +import org.jclouds.etcd.domain.keys.Key;
> +import org.jclouds.etcd.fallbacks.EtcdFallbacks.NullOnKeyNonFoundAnd404;
> +
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Path("/{jclouds.api-version}/keys")
> +public interface KeysApi {
> +
> + @Named("keys:create")
> + @PUT
> + @Path("/{key}")
> + Key createKey(@PathParam("key") String key,
> + @FormParam("value") String value,
> + @Nullable @FormParam("ttl") Integer seconds);
It would look cleaner to have an overloaded method, one without TTL and another with an `int` (primitive) TTL.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52901654
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> + * limitations under the License.
> + */
> +package org.jclouds.etcd.features;
> +
> +import static org.testng.Assert.assertNotNull;
> +import static org.testng.Assert.assertNull;
> +import static org.testng.Assert.assertTrue;
> +
> +import java.util.UUID;
> +
> +import org.jclouds.etcd.BaseEtcdApiLiveTest;
> +import org.jclouds.etcd.domain.keys.Key;
> +import org.testng.annotations.BeforeClass;
> +import org.testng.annotations.Test;
> +
> +@Test(groups = "live", testName = "KeysApiLiveTest", singleThreaded = true)
Does this really need to be single threaded?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52988637
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
Closed #234.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#event-554815902
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> + @Named("keys:create")
> + @PUT
> + @Path("/{key}")
> + Key createKey(@PathParam("key") String key,
> + @FormParam("value") String value,
> + @Nullable @FormParam("ttl") Integer seconds);
> +
> + @Named("keys:get-key")
> + @GET
> + @Path("/{key}")
> + @Fallback(NullOnKeyNonFoundAnd404.class)
> + Key getKey(@PathParam("key") String key,
> + @Nullable @QueryParam("recursive") Boolean recursive,
> + @Nullable @QueryParam("sorted") Boolean sorted);
> +
> + @Named("keys:delete-key")
Same about naming.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52901938
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.etcd.domain.keys.Key;
> +import org.testng.annotations.BeforeClass;
> +import org.testng.annotations.Test;
> +
> +import com.google.common.base.Throwables;
> +
> +@Test(groups = "live", testName = "KeysApiLiveTest")
> +public class KeysApiLiveTest extends BaseEtcdApiLiveTest {
> +
> + private String key;
> + private String value;
> +
> + @BeforeClass
> + protected void init() {
> + key = UUID.randomUUID().toString().replaceAll("-", "");
> + value = UUID.randomUUID().toString().replaceAll("-", "");
Just seen that this "key id formatting convention" is applied everywhere. Better create a static helper `randomKey()` to generate the key ids?
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r53013537
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Christopher Dancy <no...@github.com>.
@nacx for this initial PR lets keep things simple and leave those other methods out. I want to get a good foundation built and going to make the subsequent PR's easier on not only myself but others reviewing.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-184689207
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> + String localValue = UUID.randomUUID().toString().replaceAll("-", "");
> + Key createdKey = api().createKey(localKey, localValue, 1);
> + assertNotNull(createdKey);
> + assertNotNull(createdKey.node().expiration());
> +
> + try {
> + Thread.sleep(3000);
> + } catch (InterruptedException e) {
> + e.printStackTrace();
> + }
> +
> + createdKey = api().getKey(localKey);
> + assertNull(createdKey);
> + }
> +
> + @Test(dependsOnMethods = "testCreateKeyWithTTL")
This test uses different keys than the TTL one. There is no need to depend on it.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52988719
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Christopher Dancy <no...@github.com>.
@nacx latest comments should all be addressed.
On a side/separate note: any suggestions on how to force a specific live test-class to run AFTER all other live tests are completed? In this case the MembersApiLiveTest needs to run last or risk fudging up the other tests while they are attempting to modify the cluster. All of my attempts thus far have been in vain.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-184700318
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> @@ -35,4 +35,15 @@ public Boolean createOrPropagate(Throwable t) throws Exception {
> throw propagate(t);
> }
> }
> +
> + public static final class NullOnKeyNonFoundAnd404 implements Fallback<Object> {
> + public Object createOrPropagate(Throwable t) throws Exception {
> + if (checkNotNull(t, "throwable") != null &&
> + t.getMessage().contains("Could not find key") &&
> + returnValueOnCodeOrNull(t, true, equalTo(404)) != null) {
Use `HttpUtils.contains404(t)`
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52901471
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
Yes :)
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-185433561
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
Pushed to master as [7b21c2e6](http://git-wip-us.apache.org/repos/asf/jclouds-labs/commit/7b21c2e6). Thanks @cdancy!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-185626709
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> +import org.jclouds.rest.annotations.Fallback;
> +import org.jclouds.etcd.domain.keys.Key;
> +import org.jclouds.etcd.fallbacks.EtcdFallbacks.NullOnKeyNonFoundAnd404;
> +
> +@Consumes(MediaType.APPLICATION_JSON)
> +@Path("/{jclouds.api-version}/keys")
> +public interface KeysApi {
> +
> + @Named("keys:create")
> + @PUT
> + @Path("/{key}")
> + Key createKey(@PathParam("key") String key,
> + @FormParam("value") String value,
> + @Nullable @FormParam("ttl") Integer seconds);
> +
> + @Named("keys:get-key")
Use just `keys:get`, to follow the convention used in the create method (and this is already prefixed as key).
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52901763
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Christopher Dancy <no...@github.com>.
@nacx done!
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234#issuecomment-185441164
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> +public interface KeysApi {
> +
> + @Named("keys:create")
> + @PUT
> + @Path("/{key}")
> + Key createKey(@PathParam("key") String key,
> + @FormParam("value") String value,
> + @Nullable @FormParam("ttl") Integer seconds);
> +
> + @Named("keys:get-key")
> + @GET
> + @Path("/{key}")
> + @Fallback(NullOnKeyNonFoundAnd404.class)
> + Key getKey(@PathParam("key") String key,
> + @Nullable @QueryParam("recursive") Boolean recursive,
> + @Nullable @QueryParam("sorted") Boolean sorted);
Same as before. Allowing to pass those `(foo, null, null)` is kinda ugly. Consider providing an overloaded version that uses primitive types for the options.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52901914
Re: [jclouds-labs] JCLOUDS-1076: Initial implementation of Keys API
(#234)
Posted by Ignasi Barrera <no...@github.com>.
> + */
> +@Test(groups = "unit", testName = "KeysApiMockTest")
> +public class KeysApiMockTest extends BaseEtcdMockTest {
> +
> + public void testCreateKey() throws Exception {
> + MockWebServer server = mockEtcdJavaWebServer();
> +
> + server.enqueue(new MockResponse().setBody(payloadFromResource("/keys-create.json")).setResponseCode(201));
> + EtcdApi etcdApi = api(server.getUrl("/"));
> + KeysApi api = etcdApi.keysApi();
> + try {
> + Key createdKey = api.createKey("hello", "world");
> + assertNotNull(createdKey);
> + assertTrue(createdKey.node().key().equals("/hello"));
> + assertTrue(createdKey.node().value().equals("world"));
> + assertSent(server, "PUT", "/" + EtcdApiMetadata.API_VERSION + "/keys/hello");
You need to verify that the body (the form params in this case) are also properly sent.
---
Reply to this email directly or view it on GitHub:
https://github.com/jclouds/jclouds-labs/pull/234/files#r52989196