[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-21 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112797758 --- Diff: CMakeLists.txt --- @@ -145,6 +145,7 @@ if (ENABLE_VECTOR_PREDICATE_SHORT_CIRCUIT) ) endif() +option(ENABLE_NETWORK_C

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-21 Thread zuyu
Github user zuyu commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112790553 --- Diff: cli/NetworkCliClient.hpp --- @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-21 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112788365 --- Diff: cli/tests/CMakeLists.txt --- @@ -32,6 +32,17 @@ if (ENABLE_DISTRIBUTED) "${PROJECT_SOURCE_DIR}/utility/textbased_

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-21 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112788031 --- Diff: cli/NetworkCliClient.hpp --- @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or mor

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-21 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112787479 --- Diff: cli/NetworkCliClient.hpp --- @@ -0,0 +1,91 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or mor

[GitHub] incubator-quickstep pull request #236: WorkOrder proto clean-up.

2017-04-21 Thread asfgit
Github user asfgit closed the pull request at: https://github.com/apache/incubator-quickstep/pull/236 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the fe

[GitHub] incubator-quickstep issue #236: WorkOrder proto clean-up.

2017-04-21 Thread jianqiao
Github user jianqiao commented on the issue: https://github.com/apache/incubator-quickstep/pull/236 LGTM! Merging. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wis

[GitHub] incubator-quickstep issue #232: QUICKSTEP-87 Adds network cli interface.

2017-04-21 Thread cramja
Github user cramja commented on the issue: https://github.com/apache/incubator-quickstep/pull/232 I should also mention that @hbdeshmukh and I had a discussion yesterday about refactoring the CLI main method. I was unhappy with how cluttered seeming it has become, and this PR only mak

[GitHub] incubator-quickstep issue #232: QUICKSTEP-87 Adds network cli interface.

2017-04-21 Thread cramja
Github user cramja commented on the issue: https://github.com/apache/incubator-quickstep/pull/232 @zuyu Thank you for review. Regarding the question about mutexes and cond_var in the `NetworkCliServiceImpl`, we disallow multiple gRPC threads from submitting requests at the same time.

[GitHub] incubator-quickstep pull request #232: QUICKSTEP-87 Adds network cli interfa...

2017-04-21 Thread cramja
Github user cramja commented on a diff in the pull request: https://github.com/apache/incubator-quickstep/pull/232#discussion_r112686604 --- Diff: CMakeLists.txt --- @@ -145,6 +145,7 @@ if (ENABLE_VECTOR_PREDICATE_SHORT_CIRCUIT) ) endif() +option(ENABLE_NETWORK

[GitHub] incubator-quickstep issue #236: WorkOrder proto clean-up.

2017-04-21 Thread cramja
Github user cramja commented on the issue: https://github.com/apache/incubator-quickstep/pull/236 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if