Added Delete Index support to high-level REST client #27019

catalin-ursachi · 2017-10-15T23:46:32Z

Added high-level REST client support for one of the endpoints covered by #25847. Will try and add some more, if this one's fine. 🙂

(Haven't been able to get gradle check to run through yet - due to error:

:docs:integTestCluster#installAnalysisIcuPlugin FAILED

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':docs:integTestCluster#installAnalysisIcuPlugin'.
> A problem occurred starting process 'command 'cmd''

but ES builds fine, and the newly added tests pass.)

elasticmachine · 2017-10-15T23:46:34Z

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

s1monw · 2017-10-16T10:19:42Z

@elasticmachine ok to test

s1monw · 2017-10-16T10:21:03Z

@catalin-ursachi this is awesome. It's exactly how we envisioned this client to grow. We will go through the code with you soon but bear with us we have a large team meeting this week so it might be delayed a bit. Thanks for opening this PR

s1monw · 2017-10-16T13:22:24Z

@catalin-ursachi FYI you can just cd client/rest-high-level/ and rungradle check from there no need to run all tests here on you end. Also gradle precommit is useful as it allows you to only run stuff like checkstyle etc.

javanna

hi @catalin-ursachi thanks a lot for your PR! it looks good, I left some comments and requested some changes. Also, it would be nice to add some docs for this in https://github.com/elastic/elasticsearch/tree/master/docs/java-rest/high-level . Ping me if you need any help!

javanna · 2017-10-19T08:22:20Z

client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java

@@ -378,6 +390,11 @@ static String endpoint(String[] indices, String[] types, String endpoint) {
        return endpoint(String.join(",", indices), String.join(",", types), endpoint);
    }

+
+    static String endpoint(String[] indices, String endpoint) {


can you remove the endpoint argument here, as it is not needed. We will add it later if/when we need it for other APIs.

The problem is that it would then clash with the method below:

/** * Utility method to build request's endpoint. */ static String endpoint(String... parts) { StringJoiner joiner = new StringJoiner("/", "/", ""); for (String part : parts) { if (Strings.hasLength(part)) { joiner.add(part); } } return joiner.toString(); }

To be fair, having multiple endpoint methods that all take String collections as parameters, but have different behaviour, does feel like asking for trouble. How about renaming one or the other to buildEndpoint, buildPath, or similar?

Alternatively, I could just not add a new method and call:

String endpoint = endpoint(deleteIndexRequest.indices(), new String[0], "");

Would that be preferable?

javanna · 2017-10-19T08:24:09Z

client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java

+
+        Params parameters = Params.builder();
+        parameters.withTimeout(deleteIndexRequest.timeout());
+        parameters.withIndicesOptions(deleteIndexRequest.indicesOptions());


I think that master_timeout is ignored, can you read and set that one too?

javanna · 2017-10-19T08:25:25Z

client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java

@@ -347,6 +349,26 @@ public void deleteAsync(DeleteRequest deleteRequest, ActionListener<DeleteRespon
    }

    /**
+     * Deletes an index using the Delete Index api
+     * <p>
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-delete-index.html">Delete API on elastic.co</a>


can you replace here in the javadocs Delete API with Delete Index API?

javanna · 2017-10-19T08:25:48Z

client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java

+    /**
+     * Asynchronously deletes an index using the Delete Index api
+     * <p>
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-delete-index.html">Delete API on elastic.co</a>


same as above: Delete API -> Delete Index API?

javanna · 2017-10-19T08:32:56Z

client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java

+     */
+    public void deleteIndexAsync(DeleteIndexRequest deleteIndexRequest, ActionListener<DeleteIndexResponse> listener, Header... headers) {
+        performRequestAsyncAndParseEntity(deleteIndexRequest, Request::deleteIndex, DeleteIndexResponse::fromXContent, listener,
+            Collections.singleton(404), headers);


the ignore set is needed for response parsing, in case an error code is returned but the corresponding response body needs to be parsed into a response object rather than an exception. That is needed for the GET API for instance, as 404 corresponds to a valid GetResponse being returned with found set to false. IN this case, 404 corresponds to an error which needs to be parsed into an exception, so I don't think we need to add 404 to the ignore set, such set should be empty. Makes sense?

javanna · 2017-10-19T10:09:02Z

client/rest-high-level/src/test/java/org/elasticsearch/client/IndexAdminIT.java

+
+public class IndexAdminIT extends ESRestHighLevelClientTestCase {
+
+    public void testDeleteIndex_ifIndexExists() throws IOException {


I would probably merge the two methods into a single testDeleteIndex method

javanna · 2017-10-19T10:19:11Z

core/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java

@@ -61,4 +68,41 @@ protected void readAcknowledged(StreamInput in) throws IOException {
    protected void writeAcknowledged(StreamOutput out) throws IOException {
        out.writeBoolean(acknowledged);
    }
+
+    @Override
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {


I think that this is a potentially good change, but should not be part of this PR as it's not really needed. Also, if we go down this route, we should move code from AcknowledgedRestListener#buildResponse to AcknowledgedResponse and change the way RestDeleteIndexAction prints the response body out, otherwise we introduce code duplication. Would you mind doing this in another PR and reverting it here?

Ah, sorry, that was just some over-enthusiastic copy & pasting.

no worries!

javanna · 2017-10-20T07:46:37Z

client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java

+    static String endpoint(String[] indices, String endpoint) {
+        return endpoint(String.join(",", indices), endpoint);
+    }
+
    /**
     * Utility method to build request's endpoint.
     */


Can we add a unit test method to RequestTests for the new deleteIndex method?

javanna · 2017-10-20T07:48:21Z

core/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexResponse.java

@@ -48,4 +51,34 @@ public void writeTo(StreamOutput out) throws IOException {
        super.writeTo(out);
        writeAcknowledged(out);
    }
+
+    public static DeleteIndexResponse fromXContent(XContentParser parser) throws IOException {


We need to add unit tests for this parsing method, similar to what we did in IndexResponseTests, GetResponseTests etc.

javanna · 2017-10-20T07:50:03Z

core/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java

+    }
+
+    protected static void parseInnerToXContent(XContentParser parser, AcknowledgedResponse.Builder context) throws IOException {
+        XContentParser.Token token = parser.currentToken();


given this hierarchy, maybe we should check already if we support classes that add their custom fields to the response, like CreateIndexResponse. Maybe we can add parsing code for that one too (only parsing, without adding the rest of the API)? What do you think?

catalin-ursachi · 2017-10-20T10:22:24Z

Thanks for the review; I'll write the changes as soon as I get the time (possibly Sunday). 🙂

Added CreateIndexResponse fromXContent method

catalin-ursachi · 2017-10-22T15:29:37Z

client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java

@@ -378,6 +390,10 @@ static String endpoint(String[] indices, String[] types, String endpoint) {
        return endpoint(String.join(",", indices), String.join(",", types), endpoint);
    }

+    static String endpoint(String[] indices, String endpoint) {


(see comment thread: #27019 (comment))

thanks for your comments, how about using the existing endpoint method then and calling it like this endpoint(indices, Strings.EMPTY_ARRAY, "") ?

catalin-ursachi · 2017-10-22T15:37:58Z

Addressed all the review comments (still unsure about #27019 (comment), however). Will write the docs shortly.

javanna · 2017-10-23T14:28:58Z

client/rest-high-level/src/main/java/org/elasticsearch/client/Request.java

@@ -378,6 +390,10 @@ static String endpoint(String[] indices, String[] types, String endpoint) {
        return endpoint(String.join(",", indices), String.join(",", types), endpoint);
    }

+    static String endpoint(String[] indices, String endpoint) {


thanks for your comments, how about using the existing endpoint method then and calling it like this endpoint(indices, Strings.EMPTY_ARRAY, "") ?

javanna · 2017-10-23T14:34:14Z

client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java

+ *
+ * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/5.6/indices.html">Indices API on elastic.co</a>
+ */
+public class IndicesClient {


should we make this final?

javanna · 2017-10-23T14:35:03Z

client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java

@@ -221,6 +221,15 @@ public final void close() throws IOException {
    }

    /**
+     * Provides an {@link IndicesClient} which can be used to access the Indices API.
+     *
+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/5.6/indices.html">Indices API on elastic.co</a>


can you replace the version number with current like in the other links in this class?

javanna · 2017-10-23T14:35:21Z

client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java

+/**
+ * A wrapper for the {@link RestHighLevelClient} that provides methods for accessing the Indices API.
+ *
+ * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/5.6/indices.html">Indices API on elastic.co</a>


can you replace 5.6 with current?

javanna · 2017-10-23T14:38:31Z

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java

+    }
+
+    private void createIndex(String index) throws IOException {
+        Response response = client().performRequest("PUT", "/" + index);


I guess you don't need the "/" in front of the index name here. the client will do it for you if I remember correctly

javanna · 2017-10-23T14:39:46Z

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java

+        assertEquals(200, response.getStatusLine().getStatusCode());
+    }
+
+    private boolean indexExists(String index) throws IOException {


can we make this static?

javanna · 2017-10-23T14:39:54Z

client/rest-high-level/src/test/java/org/elasticsearch/client/IndicesClientIT.java

+        }
+    }
+
+    private void createIndex(String index) throws IOException {


can this be static?

javanna · 2017-10-23T14:47:21Z

core/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java

@@ -61,4 +68,41 @@ protected void readAcknowledged(StreamInput in) throws IOException {
    protected void writeAcknowledged(StreamOutput out) throws IOException {
        out.writeBoolean(acknowledged);
    }
+
+    @Override
+    public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException {


no worries!

javanna

hi @catalin-ursachi I did another review round and left a bunch of comments. Most of them are really minor though, this looks great already! Thanks a lot!

javanna · 2017-10-23T15:20:33Z

client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java

+     * See <a href="https://www.elastic.co/guide/en/elasticsearch/reference/5.6/indices.html">Indices API on elastic.co</a>
+     */
+    public IndicesClient indices() {
+        return new IndicesClient(this);


can you eagerly instantiate this and return always the same instance? What's the advantage of creating a new instance every time?

javanna · 2017-10-23T15:28:44Z

client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java

+            deleteIndexRequest.indicesOptions(IndicesOptions.fromOptions(randomBoolean(), randomBoolean(), randomBoolean(),
+                randomBoolean()));
+        }
+        expectedParams.put("ignore_unavailable", Boolean.toString(deleteIndexRequest.indicesOptions().ignoreUnavailable()));


do you want to share this same piece of code with testSearch by making a common private method?

That's what I intended; however, the DeleteIndexRequest and the SearchRequest don't share any superclass; they define the indicesOptions methods separately. So a common method would have to take getter and setter lambdas as arguments; would that be preferable?

I like the lambdas idea, maybe you can do the same with timeout.

javanna · 2017-10-23T15:39:36Z

core/src/main/java/org/elasticsearch/action/support/master/AcknowledgedResponse.java

@@ -61,4 +66,10 @@ protected void readAcknowledged(StreamInput in) throws IOException {
    protected void writeAcknowledged(StreamOutput out) throws IOException {
        out.writeBoolean(acknowledged);
    }
+
+    protected static final ParseField ACKNOWLEDGED = new ParseField("acknowledged");


does this need to be protected?

Indeed it doesn't.

javanna · 2017-10-23T15:48:03Z

core/src/main/java/org/elasticsearch/action/admin/indices/create/CreateIndexResponse.java

+    private static final ParseField INDEX = new ParseField("index");
+
+    private static final ConstructingObjectParser<CreateIndexResponse, Void> PARSER = new ConstructingObjectParser<>("create_index",
+        true, a -> new CreateIndexResponse((boolean) a[0], (boolean) a[1], (String) a[2]));


maybe rename a to args?

javanna · 2017-10-23T15:48:09Z

core/src/main/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexResponse.java

+    }
+
+    private static final ConstructingObjectParser<DeleteIndexResponse, Void> PARSER = new ConstructingObjectParser<>("delete_index",
+        true, a -> new DeleteIndexResponse((boolean) a[0]));


maybe rename a to args here too?

javanna · 2017-10-23T16:04:21Z

core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexResponseTests.java

+     * The left element is the actual {@link XContentBuilder} to serialize while the right element is the
+     * expected {@link CreateIndexResponse} after parsing.
+     */
+    public static Tuple<XContentBuilder, CreateIndexResponse> randomCreateIndexResponse(


rename this to createTestItem? also, can it be private?

javanna · 2017-10-23T16:51:21Z

core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexResponseTests.java

+        builder.field("acknowledged", acknowledged);
+        builder.field("shards_acknowledged", shardsAcked);
+        builder.field("index", index);
+        builder.endObject();


for testing purposes, I guess it makes sense to make CreateIndexResponse and DeleteIndexResponse implement ToXContentObject (part of what I made you revert, sorry!). You could also add a TODO that explains that once all of the AcknowledgedResponses are migrated we can update the rest listener accordingly.

With such change we can then use toShuffledXContent and make this method return a CreateIndexResponse rather than a Tuple.

We could then add a simple testToXContent method as well like we have in GetResponseTests and friends.

I'd be tempted to add a TODO here, asking for the test to be updated once AcknowledgedResponses implement ToXContent. Would that be okay, or would you rather I changed 'Create Index' and 'Delete Index' to implement ToXContent now?

Never mind the above; I've modified CreateIndexResponse and DeleteIndexResponse to implement ToXContentObject, and added a TODO in AcknowledgedRestListener.java.

++ thanks I think there is some value in this from a testing point of view, thank you!

javanna · 2017-10-23T16:52:59Z

core/src/test/java/org/elasticsearch/action/admin/indices/create/CreateIndexResponseTests.java

+        assertCreateIndexResponse(expectedCreateIndexResponse, parsedCreateIndexResponse);
+    }
+
+    public static void assertCreateIndexResponse(CreateIndexResponse expected, CreateIndexResponse actual) {


does this need to be public?

actually, do we even need a method for this given that we call it only once?

javanna · 2017-10-23T16:55:40Z

core/src/test/java/org/elasticsearch/action/admin/indices/delete/DeleteIndexResponseTests.java

+     * The left element is the actual {@link XContentBuilder} to serialize while the right element is the
+     * expected {@link DeleteIndexResponse} after parsing.
+     */
+    public static Tuple<XContentBuilder, DeleteIndexResponse> randomDeleteIndexResponse(


I would align this to CreateIndexResponse once you updated it according to my comments above, same direction here too.

javanna · 2017-10-23T17:14:25Z

client/rest-high-level/src/main/java/org/elasticsearch/client/IndicesClient.java

+    }
+
+    /**
+     * Asynchronously deletes an index using the Delete Index api


can you make api capital in all of its occurrences please?

javanna · 2017-10-24T10:34:30Z

retest this please

javanna

This looks great, thanks a lot for your work @catalin-ursachi !!! I left a couple of minor comments, but those are the last things I could find, and very small ones. I will merge this once you addressed those.

javanna · 2017-10-24T10:08:12Z

client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java

@@ -281,6 +269,8 @@ public void testDeleteIndex() throws IOException {
        assertNull(request.getEntity());
    }

+
+


nit: may I ask you to remove these two empty lines?

javanna · 2017-10-24T10:12:47Z

client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java

+            expectedParams.put("expand_wildcards", "closed");
+        } else {
+            expectedParams.put("expand_wildcards", "none");
+        }


I wonder if we should simplify this method. Given that we generate the random indices options as part of the method, we could reuse those rather than using the getter, then the getter argument is not needed. Also you could just return the indices options and that would remove the need for the setter argument too. Thoughts?

But if we don't generate an IndicesOptions, the getter then gets the default for the given request. Without that, we'd have to pass in the default to this method.

So, we could change it to:

private static IndicesOptions createAndExpectRandomIndicesOptions( IndicesOptions default, Map<String, String> expectedParams) { if (randomBoolean()) { ... } else { //use default return null; }

And then handle the returned value depending on whether it's null or not. Is that nicer, though?

good point! I missed the default case. I do not like the null invariant that we would introduce here, leave it as-is. Thanks for your patience ;)

javanna · 2017-10-24T10:14:38Z

client/rest-high-level/src/main/java/org/elasticsearch/client/RestHighLevelClient.java

@@ -365,7 +376,7 @@ public void searchAsync(SearchRequest searchRequest, ActionListener<SearchRespon
    }

    /**
-     * Executes a search using the Search Scroll api
+     * Executes a search using the Search Scroll API


good catch :) thanks for adjusting these

javanna · 2017-10-24T10:15:23Z

client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java

+    }
+
+
+


may I ask you to remove these empty lines and leave just one?

javanna · 2017-10-24T10:25:18Z

docs/java-rest/high-level/apis/deleteindex.asciidoc

+include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[delete-index-request-timeout]
+--------------------------------------------------
+<1> Timeout to wait for primary shard to become available as a `TimeValue`
+<2> Timeout to wait for primary shard to become available as a `String`


This is rather the "Timeout to wait for the all the nodes to acknowledge the index deletion"

javanna · 2017-10-24T10:26:51Z

docs/java-rest/high-level/apis/deleteindex.asciidoc

+<2> Called in case of failure. The raised exception is provided as an argument
+
+[[java-rest-high-delete-index-response]]
+==== Delete Response


make it "Delete Index Response" ?

javanna · 2017-10-24T10:27:26Z

docs/java-rest/high-level/apis/deleteindex.asciidoc

+[[java-rest-high-delete-index-response]]
+==== Delete Response
+
+The returned `DeleteResponse` allows to retrieve information about the executed


DeleteIndexResponse rather than DeleteResponse

javanna · 2017-10-24T10:31:02Z

docs/java-rest/high-level/apis/deleteindex.asciidoc

+
+["source","java",subs="attributes,callouts,macros"]
+--------------------------------------------------
+include-tagged::{doc-tests}/IndicesClientDocumentationIT.java[delete-index-response]


could you add a little bullet point explaining what the acknowledged field holds? "Indicates whether all of the nodes have acknowledged the request or not"

javanna · 2017-10-24T10:39:45Z

docs/java-rest/high-level/apis/index.asciidoc

@@ -1,4 +1,5 @@
 include::_index.asciidoc[]
+include::deleteindex.asciidoc[]


can we move this one line above?

javanna · 2017-10-24T13:22:27Z

client/rest-high-level/src/test/java/org/elasticsearch/client/RequestTests.java

+        }
+    }
+
+    private static <T> void setRandomTimeout(Function<String, T> setter, TimeValue defaultTimeout, Map<String, String> expectedParams) {


here and in setRandomIndicesOptions, you could replace the Function with a Consumer. That works and makes the generics go away.

catalin-ursachi · 2017-10-24T19:29:00Z

The failure looks unrelated to the changes; could you please retest?

tvernum · 2017-10-25T04:16:45Z

@elasticmachine test this please

javanna · 2017-10-25T06:28:19Z

retest this please

…hlrc

catalin-ursachi · 2017-10-25T15:40:27Z

No idea if merging in master fixed this (had it become incompatible with some external dependency?), or it was just a series of flukes; but, anyway.

javanna · 2017-10-25T19:03:15Z

@catalin-ursachi I looked at the failures too today and I am not sure what happened there, something broke badly but it was not certainly your fault, merging master in fixed it, happy that the build is green now, I will merge this in tomorrow.

javanna · 2017-10-26T07:53:22Z

Thanks a lot @catalin-ursachi , great job!

Relates to #25847

Added Delete Index support to high-level REST client

e0345a8

javanna self-requested a review October 16, 2017 10:40

javanna added :Java High Level REST Client >feature labels Oct 16, 2017

catalin-ursachi added 2 commits October 16, 2017 12:08

Fixed code style

1540e93

Fixed remaining checkstyle errors

c8e35a6

javanna requested changes Oct 20, 2017

View reviewed changes

catalin-ursachi added 2 commits October 22, 2017 14:03

Review mark-ups

77a9939

Exctracted IndicesClient from RestHighLevelClient;

a18662f

Added CreateIndexResponse fromXContent method

catalin-ursachi commented Oct 22, 2017

View reviewed changes

Fixed typos

a03b759

Added Delete Index docs

30319ea

javanna reviewed Oct 23, 2017

View reviewed changes

javanna requested changes Oct 23, 2017

View reviewed changes

catalin-ursachi added 2 commits October 23, 2017 21:23

Added Delete Index docs; review mark-ups

7261ac9

Use lambdas in RequestTests

1fe19fe

javanna added v7.0.0 v6.1.0 labels Oct 24, 2017

javanna approved these changes Oct 24, 2017

View reviewed changes

javanna reviewed Oct 24, 2017

View reviewed changes

Mark-ups

ec05d37

javanna reviewed Oct 24, 2017

View reviewed changes

catalin-ursachi added 2 commits October 24, 2017 18:39

Simplify lambda

63ba5b5

Simplified lambda

afdcb40

catalin-ursachi added 2 commits October 25, 2017 13:00

Deleted redundant type parameter

c8620a9

Merge remote-tracking branch 'origin/master' into 25847-delete-index-…

dda2d39

…hlrc

javanna merged commit 8bf3324 into elastic:master Oct 26, 2017

javanna pushed a commit that referenced this pull request Oct 26, 2017

Add Delete Index API support to high-level REST client (#27019)

eac41dd

Relates to #25847

javanna mentioned this pull request Nov 1, 2017

Java high-level REST client completeness #27205

Closed

80 tasks

colings86 added v7.0.0-beta1 and removed v7.0.0 labels Feb 7, 2019


		public class IndexAdminIT extends ESRestHighLevelClientTestCase {

		public void testDeleteIndex_ifIndexExists() throws IOException {

		@@ -281,6 +269,8 @@ public void testDeleteIndex() throws IOException {
		assertNull(request.getEntity());
		}

		@@ -1,4 +1,5 @@
		include::_index.asciidoc[]
		include::deleteindex.asciidoc[]

Added Delete Index support to high-level REST client #27019

Added Delete Index support to high-level REST client #27019

Conversation

catalin-ursachi commented Oct 15, 2017 • edited Loading

elasticmachine commented Oct 15, 2017

s1monw commented Oct 16, 2017

s1monw commented Oct 16, 2017

s1monw commented Oct 16, 2017

javanna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catalin-ursachi commented Oct 20, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catalin-ursachi commented Oct 22, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javanna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catalin-ursachi Oct 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catalin-ursachi Oct 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

javanna commented Oct 24, 2017

javanna left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catalin-ursachi Oct 24, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

catalin-ursachi commented Oct 24, 2017

tvernum commented Oct 25, 2017

javanna commented Oct 25, 2017

catalin-ursachi commented Oct 25, 2017

javanna commented Oct 25, 2017 • edited Loading

javanna commented Oct 26, 2017

catalin-ursachi commented Oct 15, 2017 •

edited

Loading

catalin-ursachi Oct 23, 2017 •

edited

Loading

catalin-ursachi Oct 23, 2017 •

edited

Loading

catalin-ursachi Oct 24, 2017 •

edited

Loading

javanna commented Oct 25, 2017 •

edited

Loading