Implement create space, drop space, add hosts, show hosts , remove hosts via MetaClient #223

ayyt · 2019-03-21T12:36:45Z

#200 Support some admin sentences in Query Engine.
Implement the following SQL via MetaClient.：
create space
drop space
add hosts
show hosts
remove hosts

nebula-community-bot · 2019-03-21T12:37:04Z

Can one of the admins verify this patch?

dutor · 2019-03-25T00:06:47Z

src/graph/AdminExecutor.cpp

+
+    switch (show_kind) {
+        case ShowKind::kShowHosts:
+            InitMetaClient();


About using the MetaClient, here are my thoughts:

You just should not create a meta client per action, not even per request. As for the current infra, the ExecutionEngine ought to create and own it, and each ExecutionContext shall have a reference to it via a raw pointer.

We disallow any synchronous action in the service, e.g. RPC. Here if the meta client doesn't support the async calls, we have to adjust it to do so.

Excellent! I will do it.

dutor · 2019-03-25T00:18:50Z

src/parser/parser.yy

+    ;
+
+create_space_sentence
+    : KW_CREATE KW_SPACE L_PAREN STRING COMMA INTEGER COMMA INTEGER R_PAREN {


I opt to define the syntax like this:

KW_CREATE KW_SPACE LABEL L_PAREN space_opt_list R_PAREN space_opt_list: space_opt_item | space_opt_list COMMA space_opt_item | space_opt_list COMMA space_opt_item : KW_ENGINE_TYPE ASSIGN LABEL | KW_PARTITION_NUM ASSIGN INTEGER | ...

dutor · 2019-03-26T05:58:12Z

@steppenwolfyuetong For your information, @dangleptr is working on the async interfaces of MetaClient

ayyt · 2019-03-26T07:50:54Z

Fair enough!

ayyt · 2019-03-26T13:08:38Z

reconstruct create space and ExecutionEngine could create and own metaClient.
meta client async interfaces need to wait for #238

dangleptr · 2019-04-01T03:10:42Z

src/graph/AdminExecutor.cpp

+void CreateSpaceExecutor::execute() {
+    CHECK_GT(partNum_, 0) << "partition_num value illegal";
+    CHECK_GT(replicaFactor_, 0) << "replica_factor value illegal";
+    auto ret = ectx()->getMetaClient()->createSpace(*spaceName_, partNum_, replicaFactor_).get();


You should use the callback here, don't block the worker.

👍 I will do it.

dangleptr · 2019-04-01T03:13:05Z

src/meta/processors/DropSpaceProcessor.cpp

+    while (iter->valid()) {
+        auto key = iter->key();
+        resp_.set_code(cpp2::ErrorCode::SUCCEEDED);
+        kvstore_->asyncRemove(kDefaultSpaceId_, kDefaultPartId_, key.toString(),


Use removeRange here maybe more simple.

Excellent! I will do it.

dangleptr · 2019-04-01T03:13:28Z

src/meta/processors/RemoveHostsProcessor.cpp

+
+    for (auto& hostKey : hostsKey) {
+        resp_.set_code(cpp2::ErrorCode::SUCCEEDED);
+        kvstore_->asyncRemove(kDefaultSpaceId_, kDefaultPartId_, hostKey,


Here I think not use removeRange.
Because the hosts to be remove are not a range, are a list.

dangleptr · 2019-04-01T03:14:57Z

src/meta/processors/RemoveHostsProcessor.cpp

+    }
+
+    resp_.set_code(cpp2::ErrorCode::SUCCEEDED);
+    onFinished();


When you call onFinish, the instance of current processor will be destroyed. So if your process procedure is async, onFinish should be called in callback.

Excellent! I will do it.

dangleptr · 2019-04-04T04:38:09Z

src/graph/ExecutionEngine.cpp

@@ -15,6 +15,10 @@ namespace graph {
 ExecutionEngine::ExecutionEngine() {
    schemaManager_ = std::make_unique<SchemaManager>();
    storage_ = std::make_unique<StorageService>(schemaManager_.get());
+


Add a TODO comment here , schemaManager/StorageClient should shared one meta client instance in the future

dangleptr · 2019-04-04T04:56:18Z

src/meta/processors/BaseProcessor.inl

@@ -84,6 +84,29 @@ StatusOr<GraphSpaceID> BaseProcessor<RESP>::spaceExist(const std::string& name)
    return Status::SpaceNotFound();
 }

+// TODO(YT) Maybe we could use index to improve the efficient
+template<typename RESP>
+Status BaseProcessor<RESP>::hostsExist(std::vector<std::string> &hostsKey) {


If you don't modify "hostKey", pass const reference is a better choice.

Good catch！👍

dangleptr · 2019-04-04T04:56:36Z

src/meta/processors/BaseProcessor.inl

+// TODO(YT) Maybe we could use index to improve the efficient
+template<typename RESP>
+Status BaseProcessor<RESP>::hostsExist(std::vector<std::string> &hostsKey) {
+    for (auto& hostKey : hostsKey) {


Use const auto&

Good catch！

dangleptr · 2019-04-04T05:04:58Z

src/meta/processors/BaseProcessor.inl

@@ -84,6 +84,29 @@ StatusOr<GraphSpaceID> BaseProcessor<RESP>::spaceExist(const std::string& name)
    return Status::SpaceNotFound();
 }

+// TODO(YT) Maybe we could use index to improve the efficient


The hostKey is rowKey now, i think we don't need another index here.

Yeah, it Is my negligence.

dangleptr · 2019-04-04T05:07:09Z

src/meta/processors/DropSpaceProcessor.cpp

+namespace meta {
+
+void DropSpaceProcessor::process(const cpp2::DropSpaceReq& req) {
+    guard_ = std::make_unique<std::lock_guard<std::mutex>>(


for the lock, i have optimized it in #225
I will merge it recently, you'd better rebase on it.

dangleptr · 2019-04-04T05:12:24Z

src/meta/processors/DropSpaceProcessor.cpp

+                          [this] (kvstore::ResultCode code, HostAddr leader) {
+        UNUSED(leader);
+        this->resp_.set_code(to(code));
+        this->onFinished();


It's dangerous here. Because we can't ensure the callback order for the two asyncRemove

Excellent! I will fix it.

dangleptr · 2019-04-04T05:17:11Z

src/meta/processors/DropSpaceProcessor.cpp

+                          [this] (kvstore::ResultCode code, HostAddr leader) {
+        UNUSED(leader);
+        this->resp_.set_code(to(code));
+        this->onFinished();


It's dangerous here. Because we can't ensure the callback order for the two asyncRemove

sherman-the-tank · 2019-04-08T12:27:21Z

jenkins go

ayyt · 2019-04-08T12:47:26Z

need to deal with some problems caused by the merger, and request review later

nebula-community-bot · 2019-04-08T13:34:02Z

Unit testing failed.

ayyt · 2019-04-08T13:39:59Z

The update has been completed, please review。

dangleptr · 2019-04-09T02:41:08Z

Ready to review now?

ayyt · 2019-04-09T02:51:46Z

@dangleptr, yeah， please focus on the merge in yesterday.

dangleptr · 2019-04-09T02:59:10Z

src/graph/AdminExecutor.cpp

+Status AddHostsExecutor::prepare() {
+    host_ = sentence_->hosts();
+    if (host_.size() == 0) {
+        LOG(FATAL) << "Add hosts Sentence host address illegal";


FATAL means core dump. We'd better return ErrorCode here.

Good point.

dangleptr · 2019-04-09T03:00:04Z

src/graph/AdminExecutor.cpp

+Status RemoveHostsExecutor::prepare() {
+    host_ = sentence_->hosts();
+    if (host_.size() == 0) {
+        LOG(FATAL) << "Remove hosts Sentence host address illegal";


Good catch.

dangleptr · 2019-04-09T03:02:49Z

src/graph/AdminExecutor.cpp

+
+
+void CreateSpaceExecutor::execute() {
+    CHECK_GT(partNum_, 0) << "partition_num value illegal";


You'd better check the numbers in prepare and return errorCode if failed.

👍，good catch！

dangleptr · 2019-04-09T03:03:26Z

src/graph/AdminExecutor.cpp

+
+    auto cb = [this] (auto &&resp) {
+        auto spaceId = resp.value();
+        if (spaceId == 0) {


Why we need the if scope?

Here I will use if (spaceId <= 0) to ensure that return a valid spaceId.

dangleptr · 2019-04-09T03:08:47Z

src/meta/processors/AddTagProcessor.cpp

-        } catch (std::exception& e) {
-            LOG(ERROR) << "Convert failed for " << val << ", msg " << e.what();
-        }
+       return *reinterpret_cast<const TagID*>(val.c_str());


Generally, we prefer folly::to than cast by ourself. folly::to has error checks inside, it is important.

Yeah，I see what you mean.
But storing to kvstore when creating space and getting from kvstore when droping space should be consistent，Otherwise an error occurs when getting. Also to Tag.

If here using folly::to, storing to kvstore also uses folly::to, not cast by ourself.

dangleptr · 2019-04-09T07:10:23Z

src/meta/processors/DropSpaceProcessor.cpp

+    std::string end = MetaUtils::partKey(spaceId, 0x7FFFFFFF);
+    resp_.set_code(cpp2::ErrorCode::SUCCEEDED);
+
+    kvstore_->asyncRemoveRange(kDefaultSpaceId_, kDefaultPartId_, start, end,


I reconsider the multi delete here. I think we should implement a batchDelete interface in kvstore. You could refer rocksdb::WriteBatch to implement it (Open another issue for it). wdyt?

make sense.

We can't use asyncXXX in this way. Currently, to keep safe, you should wait for all asyncXXX finished before invoke onFinished method.

Yeah, here use multi asyncXXX and collectAll.
batchDelete need to implement for remove hosts;

dangleptr · 2019-04-09T07:10:44Z

src/meta/processors/RemoveHostsProcessor.cpp

+
+    for (auto& hostKey : hostsKey) {
+        resp_.set_code(cpp2::ErrorCode::SUCCEEDED);
+        kvstore_->asyncRemove(kDefaultSpaceId_, kDefaultPartId_, hostKey,


dangleptr · 2019-04-09T07:11:13Z

src/parser/AdminSentences.cpp

+
+std::string ShowSentence::toString() const {
+    std::string buf;
+    switch (showKind_) {


buf.reserve

dangleptr · 2019-04-09T07:12:24Z

src/parser/AdminSentences.cpp

+    char buf[256];
+    switch (optType_) {
+        case PARTITION_NUM:
+            snprintf(buf, sizeof(buf), "partition_num = %ld", boost::get<int64_t>(optValue_));


You could use folly::stringPrintf directly.

Good point!

ayyt · 2019-04-09T10:06:38Z

fixed, please review again

sherman-the-tank · 2019-04-09T16:59:31Z

jenkins go

nebula-community-bot · 2019-04-09T17:16:01Z

Unit testing passed.

sherman-the-tank

Well done! Thanks for taking care of this

I have some inline comments

sherman-the-tank · 2019-04-09T17:03:37Z

src/graph/AdminExecutor.cpp

+
+using nebula::network::NetworkUtils;
+
+ShowExecutor::ShowExecutor(Sentence *sentence,


I would highly recommend to break this file into multiple files, with each file contains one Executor implementation

Generally speaking, we want to keep every class in its own header file and implementation file, unless for those accessory classes

Good, this makes sense.

sherman-the-tank · 2019-04-09T17:07:07Z

src/graph/AdminExecutor.cpp

+
+
+void ShowExecutor::execute() {
+    auto show_kind = sentence_->showKind();


We don't use C style variable name, please rename it to showKind

In addition, I discussed with @dutor this afternoon, it seems we use kind pretty much across the entire QE. But in American English, "type" is more preferable over "kind"

Right on！👍，I will fix it.

sherman-the-tank · 2019-04-09T17:13:05Z

src/graph/test/DefineSchemaTest.cpp

+        cpp2::ExecutionResponse resp;
+        std::string query = "add hosts(\"127.0.0.1:1000\", \"127.0.0.1:1100\")";
+        auto code = client->execute(query, resp);
+        ASSERT_EQ(cpp2::ErrorCode::SUCCEEDED, code);


I don't think you need ASSERT_EQ() here. EXPECT_EQ() could be more suitable

ASSERT_EQ() will stop running all following code, while EXPECT_EQ() will only print an error message and continue the testcase execusion

Right on!
But I check the reference manual about *_EQ, the rule has changed. As follow：
Historical note: Before February 2016 *_EQ had a convention of calling it as ASSERT_EQ(expected, actual), so lots of existing code uses this order. Now *_EQ treats both parameters in the same way.

refer to https://github.com/google/googletest/blob/master/googletest/docs/primer.md

IMO，we maintain the original specification as *_EQ(expected, actual)

@sherman-the-tank Here are my opinions:
From my experiences, in most cases, if an assertion(ASSERTs or EXPECTs) fails, the following testings usually make no sense. Even worse, the following testings are error-prone so that we need much more mental overheads to take care of these. So I recommend to use ASSERTs by default, unless we intentionally want the testing to continue. If you concern about that to fail earlier will print out less infos, I think we ought to take more advantages of the stream operator provided by the testing macros, e.g. ASSERT_TRUE(status.ok()) << status.

Take following codes as example:

{ cpp2::ListSpacesReq req; auto* processor = ListSpacesProcessor::instance(kv.get()); auto f = processor->getFuture(); processor->process(req); auto resp = std::move(f).get(); EXPECT_EQ(cpp2::ErrorCode::SUCCEEDED, resp.code); EXPECT_EQ(1, resp.spaces.size()); EXPECT_EQ(1, resp.spaces[0].id.get_space_id()); EXPECT_EQ("default_space", resp.spaces[0].name); }

These code will definitely crash on failure, and nobody noticed it. Besides, reviewers often pay less attention to the testing codes, at least it is true so far.

@dutor, @sherman-the-tank
I think we should use a same rule to unify all testcasts in new pr.
so future operations need to follow the same rules.
wdyt?

ayyt · 2019-04-10T04:36:09Z

fixed, please review again.

ayyt · 2019-04-11T07:32:18Z

rebase master

ayyt · 2019-04-11T10:34:50Z

rebase master

dangleptr

The patch looks more clear now. There are some inline comments, please recheck it.

dangleptr · 2019-04-12T02:08:54Z

src/meta/processors/DropSpaceProcessor.cpp

+    auto spaceRet = getSpaceId(req.get_space_name());
+
+    if (!spaceRet.ok()) {
+        if (spaceRet.status() == Status::SpaceNotFound()) {


Implement a function that convert Status to cpp2::ErrorCode

dangleptr · 2019-04-12T02:09:05Z

src/meta/processors/RemoveHostsProcessor.cpp

+
+    auto hostsRet = hostsExist(hostsKey);
+    if (!hostsRet.ok()) {
+        if (hostsRet == Status::HostNotFound()) {


dangleptr · 2019-04-12T02:10:29Z

src/parser/AdminSentences.cpp

+    buf.reserve(256);
+    switch (optType_) {
+        case PARTITION_NUM:
+            buf = folly::stringPrintf("partition_num = %ld", boost::get<int64_t>(optValue_));


"buf" seems useless. Return directly

Good catch!

dangleptr · 2019-04-12T02:11:25Z

src/parser/AdminSentences.h

+        if (isInt()) {
+            return asInt();
+        } else {
+            LOG(FATAL) << "partition_num value illegal.";


Be carefully about FATAL. Do we really want to crash here?

Good catch! 👍

dangleptr · 2019-04-12T02:11:34Z

src/parser/AdminSentences.h

+        if (isInt()) {
+            return asInt();
+        } else {
+            LOG(FATAL) << "replica_factor value illegal.";


dangleptr · 2019-04-12T02:13:32Z

src/meta/processors/DropSpaceProcessor.cpp

+    }
+
+    deleteKeys.emplace_back(MetaUtils::indexKey(EntryType::SPACE, req.get_space_name()));
+    deleteKeys.emplace_back(MetaUtils::spaceKey(spaceId));


Add TODO comments, should we delete Tag/Edge under the space here?

make sense!

dangleptr · 2019-04-12T02:16:12Z

src/meta/processors/AddTagProcessor.cpp

@@ -28,7 +28,7 @@ void AddTagProcessor::process(const cpp2::AddTagReq& req) {
    auto version = time::TimeUtils::nowInMSeconds();
    TagID tagId = autoIncrementId();
    data.emplace_back(MetaUtils::indexKey(EntryType::TAG, req.get_tag_name()),
-                      std::string(reinterpret_cast<const char*>(&tagId), sizeof(tagId)));
+                      folly::to<std::string>(tagId));


Generally, if cast from int to char*, you could cast directly. Because no errors should happen.

AddTagProcessor.cpp is same to CreatesSpaceProcessor.cpp.
`
dangleptr 3 days ago
Generally, we prefer folly::to than cast by ourself. folly::to has error checks inside, it is important.

@steppenwolfyuetong
steppenwolfyuetong 3 days ago •
Author
Yeah，I see what you mean.
But storing to kvstore when creating space and getting from kvstore when droping space should be consistent，Otherwise an error occurs when getting. Also to Tag.

@steppenwolfyuetong
steppenwolfyuetong 3 days ago Author
If here using folly::to, storing to kvstore also uses folly::to, not cast by ourself.
`

Reference to reply in CreatesSpaceProcessor.cpp

fixed according to the offline discussion

ayyt · 2019-04-12T06:10:53Z

fixed and rebase master, please review

dangleptr · 2019-04-12T07:17:34Z

Jenkins go

sherman-the-tank · 2019-04-12T14:28:53Z

jenkins go

nebula-community-bot · 2019-04-12T14:45:13Z

Unit testing passed.

ayyt · 2019-04-14T06:28:40Z

@dutor, @dangleptr, @sherman-the-tank, Thank you for your meticulous review. 👍
@dutor，I has fixed your suggestions and comments except that use default space in the prepare of executor. Let's talk about it offline on Monday, ok？

…sts via MetaClient

ayyt · 2019-04-15T08:42:14Z

@dutor, fix and rebase master, please review again, thanks.

dutor · 2019-04-15T09:16:07Z

src/parser/parser.yy

+create_space_sentence
+    : KW_CREATE KW_SPACE LABEL L_PAREN space_opt_list R_PAREN {
+        auto sentence = new CreateSpaceSentence($3);
+	    sentence->setOpts($5);


indentation.

Good catch!

src/parser/parser.yy

dutor · 2019-04-15T09:17:00Z

src/parser/parser.yy

+	    $$->addOpt($1);
+    }
+    | space_opt_list COMMA space_opt_item {
+	    $$ = $1;


indent four spaces.

dutor · 2019-04-15T09:17:10Z

src/parser/parser.yy

+	    $$->addOpt($3);
+    }
+    | space_opt_list COMMA {
+	    $$ = $1;


dutor · 2019-04-15T09:21:14Z

src/parser/parser.yy

+	    sentence->setOpts($5);
+	    $$ = sentence;
+    }
+    ;


Better to add a blank line below.

Yeah, good catch!

ayyt · 2019-04-15T09:39:47Z

@dutor, fix and rebase. thanks.

dutor · 2019-04-15T09:57:14Z

Jenkins, go!

nebula-community-bot · 2019-04-15T10:14:22Z

Unit testing passed.

dutor

…sts via MetaClient (vesoft-inc#223) close vesoft-inc#200

ayyt changed the title ~~Add host and create space~~ Implement create space, add hosts, show hosts via MetaClient Mar 21, 2019

dutor reviewed Mar 25, 2019

View reviewed changes

ayyt changed the title ~~Implement create space, add hosts, show hosts via MetaClient~~ Implement create space, add hosts, show hosts via MetaClient Mar 26, 2019

ayyt mentioned this pull request Mar 28, 2019

Implement create space、drop space、add hosts、show hosts、 remove hosts via meta client #248

Closed

ayyt changed the title ~~Implement create space, add hosts, show hosts via MetaClient~~ Implement create space, drop space, add hosts, show hosts , remove hosts via MetaClient Mar 29, 2019

dangleptr reviewed Apr 1, 2019

View reviewed changes

dangleptr reviewed Apr 4, 2019

View reviewed changes

dangleptr mentioned this pull request Apr 8, 2019

Implement GoExecutor #245

Merged

dangleptr reviewed Apr 9, 2019

View reviewed changes

sherman-the-tank reviewed Apr 9, 2019

View reviewed changes

dangleptr reviewed Apr 12, 2019

View reviewed changes

ayyt added 12 commits April 15, 2019 15:12

support add hosts

2ba91f8

Implement create space, drop space, add hosts, show hosts , remove ho…

e928491

…sts via MetaClient

reconstruct create space and rebase master

f51e941

ExecutionEngine create and own metaClient

dffaa48

Handles the inconsistency between storing and reading values

18f3dea

address dangleptr's comments

6ff1ea9

address sherman-the-tank's comments

b10cabd

address dangleptr's comments

ded88a0

use deRemove function

f250259

address dangleptr's comments

0377764

address dutor's comments

3c8879d

address dutor's comments

cc5e76c

address dutor's comments

570c0b8

dutor reviewed Apr 15, 2019

View reviewed changes

src/parser/parser.yy Outdated Show resolved Hide resolved

dutor reviewed Apr 15, 2019

View reviewed changes

src/parser/parser.yy Outdated

$$->addOpt($3);

}

| space_opt_list COMMA {

$$ = $1;

Copy link

Contributor

dutor Apr 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.

ayyt reacted with thumbs up emoji

dutor reviewed Apr 15, 2019

View reviewed changes

dutor approved these changes Apr 16, 2019

View reviewed changes

dutor merged commit b52a911 into vesoft-inc:master Apr 16, 2019

dutor deleted the addHost_and_createSpace branch April 16, 2019 01:17

yixinglu pushed a commit to yixinglu/nebula that referenced this pull request Feb 16, 2020

Implement create space, drop space, add hosts, show hosts , remove ho…

a0f43bb

…sts via MetaClient (vesoft-inc#223) close vesoft-inc#200

tong-hao pushed a commit to tong-hao/nebula that referenced this pull request Jun 1, 2021

Implement create space, drop space, add hosts, show hosts , remove ho…

409f054

…sts via MetaClient (vesoft-inc#223) close vesoft-inc#200



		void CreateSpaceExecutor::execute() {
		CHECK_GT(partNum_, 0) << "partition_num value illegal";


		using nebula::network::NetworkUtils;

		ShowExecutor::ShowExecutor(Sentence *sentence,



		void ShowExecutor::execute() {
		auto show_kind = sentence_->showKind();

Implement create space, drop space, add hosts, show hosts , remove hosts via MetaClient #223

Implement create space, drop space, add hosts, show hosts , remove hosts via MetaClient #223

Conversation

ayyt commented Mar 21, 2019 • edited Loading

nebula-community-bot commented Mar 21, 2019

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

dutor commented Mar 26, 2019

ayyt commented Mar 26, 2019

ayyt commented Mar 26, 2019

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

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

sherman-the-tank commented Apr 8, 2019

ayyt commented Apr 8, 2019

nebula-community-bot commented Apr 8, 2019

ayyt commented Apr 8, 2019

dangleptr commented Apr 9, 2019

ayyt commented Apr 9, 2019

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

ayyt Apr 9, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayyt Apr 9, 2019 • 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

ayyt commented Apr 9, 2019

sherman-the-tank commented Apr 9, 2019

nebula-community-bot commented Apr 9, 2019

sherman-the-tank 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

ayyt Apr 10, 2019 • edited Loading

Choose a reason for hiding this comment

dutor Apr 12, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ayyt commented Apr 10, 2019

ayyt commented Apr 11, 2019

ayyt commented Apr 11, 2019 • edited Loading

ayyt commented Mar 21, 2019 •

edited

Loading

ayyt Apr 9, 2019 •

edited

Loading

ayyt Apr 9, 2019 •

edited

Loading

ayyt Apr 10, 2019 •

edited

Loading

dutor Apr 12, 2019 •

edited

Loading

ayyt commented Apr 11, 2019 •

edited

Loading