[GitHub] drill issue #1037: DRILL-5968: Add support for empty service_host user prope...
Github user superbstreak commented on the issue: https://github.com/apache/drill/pull/1037 Thanks, @parthchandra ! ---
[GitHub] drill issue #1037: DRILL-5968: Add support for empty service_host user prope...
Github user superbstreak commented on the issue: https://github.com/apache/drill/pull/1037 @parthchandra We should get this in for 1.12? ---
[GitHub] drill pull request #1037: DRILL-5968: Add support for empty service_host use...
Github user superbstreak commented on a diff in the pull request: https://github.com/apache/drill/pull/1037#discussion_r151515278 --- Diff: contrib/native/client/src/include/drill/userProperties.hpp --- @@ -28,6 +28,18 @@ class DECLSPEC_DRILL_CLIENT DrillUserProperties{ static const std::map USER_PROPERTIES; DrillUserProperties(){}; + +/// @brief Update the property value associate with the property key if the value is +/// empty. +/// +/// @param in_propName The property name. +/// @param in_propValue The property value. +void setPropertyIfEmpty(const std::string& in_propName, const std::string& in_propValue){ +// If the element value is empty, then update. +if (isPropSet(in_propName) && m_properties[in_propName].empty()){ --- End diff -- certainly ---
[GitHub] drill pull request #1039: DRILL-5584: Add branding and versioning informatio...
Github user superbstreak commented on a diff in the pull request: https://github.com/apache/drill/pull/1039#discussion_r151329816 --- Diff: contrib/native/client/src/clientlib/env.h.in --- @@ -30,6 +30,11 @@ #define GIT_SHA_PROP @GIT_SHA_PROP@ #define GIT_COMMIT_PROP @GIT_COMMIT_PROP@ +#define DRILL_LEGALCOPYRIGHT_STR"Copyright (c) 2013-2017 The Apache Software Foundation\0" --- End diff -- Reviewer, please make sure this is correct ---
[GitHub] drill pull request #1039: DRILL-5584: Add branding and versioning informatio...
GitHub user superbstreak opened a pull request: https://github.com/apache/drill/pull/1039 DRILL-5584: Add branding and versioning information for windows C++ C⦠â¦lient. You can merge this pull request into a Git repository by running: $ git pull https://github.com/superbstreak/drill DRILL-5584 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/1039.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1039 commit 7792fbfa11dc9a622217eb9a8f4644802e4e5e44 Author: Rob Wu Date: 2017-11-16T02:21:48Z DRILL-5584: Add branding and versioning information for windows C++ Client. ---
[GitHub] drill pull request #1037: DRILL-5968: Add support for empty service_host use...
Github user superbstreak commented on a diff in the pull request: https://github.com/apache/drill/pull/1037#discussion_r151282197 --- Diff: contrib/native/client/src/include/drill/userProperties.hpp --- @@ -28,6 +28,20 @@ class DECLSPEC_DRILL_CLIENT DrillUserProperties{ static const std::map USER_PROPERTIES; DrillUserProperties(){}; + +/// @brief Insert or update property value associate with the property key if the value is +/// empty or key is undefined. This function is useful for setting default property. +/// +/// @param in_propName The property name. +/// @param in_propValue The property value. +void SetDefaultProperty(const std::string& in_propName, const std::string& in_propValue) --- End diff -- gotcha thanks! ---
[GitHub] drill pull request #1037: DRILL-5968: Add support for empty service_host use...
Github user superbstreak commented on a diff in the pull request: https://github.com/apache/drill/pull/1037#discussion_r151282112 --- Diff: contrib/native/client/src/include/drill/userProperties.hpp --- @@ -28,6 +28,20 @@ class DECLSPEC_DRILL_CLIENT DrillUserProperties{ static const std::map USER_PROPERTIES; DrillUserProperties(){}; + +/// @brief Insert or update property value associate with the property key if the value is +/// empty or key is undefined. This function is useful for setting default property. +/// +/// @param in_propName The property name. +/// @param in_propValue The property value. +void SetDefaultProperty(const std::string& in_propName, const std::string& in_propValue) +{ +// If the element value is empty (newly inserted or user defined), then update. +if (m_properties[in_propName].empty()) --- End diff -- If the value doesn't exist, the original implementation with the setProperty(2) would also insert a new key. Not sure if that was intended or not. ---
[GitHub] drill pull request #1037: DRILL-5968: Add support for empty service_host use...
GitHub user superbstreak opened a pull request: https://github.com/apache/drill/pull/1037 DRILL-5968: Add support for empty service_host user property You can merge this pull request into a Git repository by running: $ git pull https://github.com/superbstreak/drill DRILL-5968-Add-support-for-empty-service_host Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/1037.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #1037 commit 7deed948b05d35ff48287defad4ccefe61f0128f Author: Rob Wu Date: 2017-11-15T07:27:29Z DRILL-5968: Add support for empty service_host user property ---
[GitHub] drill issue #992: DRILL-5873: (C++ Client) Improve SASL error reporting.
Github user superbstreak commented on the issue: https://github.com/apache/drill/pull/992 @parthchandra Thanks for adding this! ---
[GitHub] drill pull request #950: DRILL-5431: SSL Support
Github user superbstreak commented on a diff in the pull request: https://github.com/apache/drill/pull/950#discussion_r140129825 --- Diff: contrib/native/client/src/include/drill/common.hpp --- @@ -163,9 +170,13 @@ typedef enum{ #define USERPROP_USERNAME "userName" #define USERPROP_PASSWORD "password" #define USERPROP_SCHEMA "schema" -#define USERPROP_USESSL "useSSL"// Not implemented yet -#define USERPROP_FILEPATH "pemLocation" // Not implemented yet -#define USERPROP_FILENAME "pemFile" // Not implemented yet +#define USERPROP_USESSL "enableTLS" +#define USERPROP_TLSPROTOCOL "TLSProtocol" //TLS version +#define USERPROP_CERTFILEPATH "certFilePath" // pem file path and name +#define USERPROP_CERTPASSWORD "certPassword" // Password for certificate file --- End diff -- I think we can remove this to avoid confusion :) ---
[GitHub] drill pull request #880: DRILL-5678: Undefined behavior due to un-initialize...
GitHub user superbstreak opened a pull request: https://github.com/apache/drill/pull/880 DRILL-5678: Undefined behavior due to un-initialized values in Server⦠â¦MetaContext You can merge this pull request into a Git repository by running: $ git pull https://github.com/superbstreak/drill DRILL-5678 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/880.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #880 commit 9ad69d060d65be3b77dc77bfa03d9d9b798cc16c Author: Rob Wu Date: 2017-07-19T05:55:52Z DRILL-5678: Undefined behavior due to un-initialized values in ServerMetaContext --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #878: DRILL-5675: Drill C++ Client reports incorrect date...
GitHub user superbstreak opened a pull request: https://github.com/apache/drill/pull/878 DRILL-5675: Drill C++ Client reports incorrect datetime Literals Sup⦠â¦port Metadata You can merge this pull request into a Git repository by running: $ git pull https://github.com/superbstreak/drill DRILL-5675 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/878.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #878 commit 3bdce2b3ba6fe032ade6e874e48ef92e292d3ade Author: Rob Wu Date: 2017-07-18T00:12:39Z DRILL-5675: Drill C++ Client reports incorrect datetime Literals Support Metadata --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #876: DRILL-5659: Fix error code checking in reading from socket
Github user superbstreak commented on the issue: https://github.com/apache/drill/pull/876 Thanks, @parthchandra! --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #873: DRILL-5668: Fix C++ connector crash on error
Github user superbstreak commented on the issue: https://github.com/apache/drill/pull/873 Thanks for the patch! --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #850: DRILL-5541: C++ Client Crashes During Simple "Man i...
GitHub user superbstreak opened a pull request: https://github.com/apache/drill/pull/850 DRILL-5541: C++ Client Crashes During Simple "Man in the Middle" Atta⦠â¦ck Test with Exploitable Write AV You can merge this pull request into a Git repository by running: $ git pull https://github.com/superbstreak/drill DRILL-5541 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/850.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #850 commit 716db51df61d0ee47804217a6a133d1d1152b64a Author: Rob Wu Date: 2017-06-05T21:06:33Z DRILL-5541: C++ Client Crashes During Simple "Man in the Middle" Attack Test with Exploitable Write AV --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #791: DRILL-5369: Add initializer for ServerMetaContext
Github user superbstreak commented on the issue: https://github.com/apache/drill/pull/791 Thanks @laurentgo for the fix! --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #772: DRILL-5316: Check drillbits size before we attempt ...
Github user superbstreak commented on a diff in the pull request: https://github.com/apache/drill/pull/772#discussion_r105046142 --- Diff: contrib/native/client/src/clientlib/drillClientImpl.cpp --- @@ -2143,6 +2146,9 @@ connectionStatus_t PooledDrillClientImpl::connect(const char* connStr, DrillUser Utils::shuffle(drillbits); // The original shuffled order is maintained if we shuffle first and then add any missing elements Utils::add(m_drillbits, drillbits); +if (m_drillbits.empty()){ +return handleConnError(CONN_FAILURE, getMessage(ERR_CONN_ZKNODBIT)); --- End diff -- Thanks, @laurentgo! A new ticket has been filed for the suggestion: [DRILL-5335](https://issues.apache.org/jira/browse/DRILL-5335) --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #772: DRILL-5316: Check drillbits size before we attempt to acce...
Github user superbstreak commented on the issue: https://github.com/apache/drill/pull/772 @sohami please review. --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #772: DRILL-5316: Check drillbitsVector count from zoo_ge...
Github user superbstreak commented on a diff in the pull request: https://github.com/apache/drill/pull/772#discussion_r104627768 --- Diff: contrib/native/client/src/clientlib/drillClientImpl.cpp --- @@ -86,6 +86,9 @@ connectionStatus_t DrillClientImpl::connect(const char* connStr, DrillUserProper std::vector drillbits; int err = zook.getAllDrillbits(hostPortStr, drillbits); if(!err){ +if (drillbits.empty()){ +return handleConnError(CONN_INVALID_INPUT, getMessage(ERR_CONN_ZKNODBIT)); --- End diff -- double check CONN_INVALID_INPUT usage. --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #772: DRILL-5316: Check drillbitsVector count from zoo_ge...
Github user superbstreak commented on a diff in the pull request: https://github.com/apache/drill/pull/772#discussion_r104607726 --- Diff: contrib/native/client/src/clientlib/zookeeperClient.cpp --- @@ -138,6 +138,11 @@ int ZookeeperClient::getAllDrillbits(const std::string& connectStr, std::vector< DRILL_MT_LOG(DRILL_LOG(LOG_TRACE) << "\t Unshuffled Drillbit id: " << drillbits[i] << std::endl;) } } +else{ --- End diff -- Thanks both. Yea make sense, I'll make the change. --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #772: DRILL-5316: Check drillbitsVector count from zoo_ge...
GitHub user superbstreak opened a pull request: https://github.com/apache/drill/pull/772 DRILL-5316: Check drillbitsVector count from zoo_get_children before ⦠â¦we attempt to access the vector element You can merge this pull request into a Git repository by running: $ git pull https://github.com/superbstreak/drill DRILL-5316 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/772.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #772 commit c9ca5a35aa684992d58d93e50a7a9ff7ce16ead8 Author: Rob Wu Date: 2017-03-07T02:17:25Z DRILL-5316: Check drillbitsVector count from zoo_get_children before we attempt to access the vector element --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #771: DRILL-5315: Address small typo in the comment
GitHub user superbstreak opened a pull request: https://github.com/apache/drill/pull/771 DRILL-5315: Address small typo in the comment You can merge this pull request into a Git repository by running: $ git pull https://github.com/superbstreak/drill DRILL-5315 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/771.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #771 commit cc23c729edfba5571ad542945dd64e5ca2055d75 Author: Rob Wu Date: 2017-03-06T22:56:14Z DRILL-5315: Address small typo in the comment --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #578: DRILL-4280: Kerberos Authentication
Github user superbstreak commented on a diff in the pull request: https://github.com/apache/drill/pull/578#discussion_r101409917 --- Diff: contrib/native/client/src/clientlib/saslAuthenticatorImpl.cpp --- @@ -0,0 +1,211 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include +#include +#include +#include "saslAuthenticatorImpl.hpp" + +#include "drillClientImpl.hpp" +#include "logger.hpp" + +namespace Drill { + +#define DEFAULT_SERVICE_NAME "drill" + +#define KERBEROS_SIMPLE_NAME "kerberos" +#define KERBEROS_SASL_NAME "gssapi" +#define PLAIN_NAME "plain" + +const std::map SaslAuthenticatorImpl::MECHANISM_MAPPING = boost::assign::map_list_of +(KERBEROS_SIMPLE_NAME, KERBEROS_SASL_NAME) +(PLAIN_NAME, PLAIN_NAME) +; + +boost::mutex SaslAuthenticatorImpl::s_mutex; +bool SaslAuthenticatorImpl::s_initialized = false; + +SaslAuthenticatorImpl::SaslAuthenticatorImpl(const DrillUserProperties* const properties) : +m_properties(properties), m_pConnection(NULL), m_pwd_secret(NULL) { + +if (!s_initialized) { +boost::lock_guard lock(SaslAuthenticatorImpl::s_mutex); +if (!s_initialized) { +// set plugin path if provided +if (DrillClientConfig::getSaslPluginPath()) { +char *saslPluginPath = const_cast(DrillClientConfig::getSaslPluginPath()); +sasl_set_path(0, saslPluginPath); +} + +// loads all the available mechanism and factories in the sasl_lib referenced by the path +const int err = sasl_client_init(NULL); +if (0 != err) { +std::stringstream errMsg; +errMsg << "Failed to load authentication libraries. code: " << err; +DRILL_MT_LOG(DRILL_LOG(LOG_TRACE) << errMsg << std::endl;) --- End diff -- This would not compile. Try errMsg.str() On windows, Im getting: Error 443 error C2678: binary '<<' : no operator found which takes a left-hand operand of type 'std::ostream' (or there is no acceptable conversion) --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #733: DRILL-5221: Send cancel message as soon as possible in C++...
Github user superbstreak commented on the issue: https://github.com/apache/drill/pull/733 Did some testing. This allow query to cancel more precisely in certain flows. Good. --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #698: DRILL-5136: Server unable to prepare non SELECT queries
Github user superbstreak commented on the issue: https://github.com/apache/drill/pull/698 Closing PR due to widen scope. --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #698: DRILL-5136: Server unable to prepare non SELECT que...
Github user superbstreak closed the pull request at: https://github.com/apache/drill/pull/698 --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill pull request #698: DRILL-5136: Server unable to prepare non SELECT que...
GitHub user superbstreak opened a pull request: https://github.com/apache/drill/pull/698 DRILL-5136: Server unable to prepare non SELECT queries Currently, the server makes every incoming queries a limit 0 query during server prepare. This is causing some non select queries to fail. We need to add an additional check to determine if the query is a SELECT query before wrapping it with LIMIT 0. You can merge this pull request into a Git repository by running: $ git pull https://github.com/superbstreak/drill DRILL-5136 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/drill/pull/698.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #698 commit 1856ca01c68ae841d90f8ed27ec577c708cec4f7 Author: Rob Wu Date: 2016-12-19T22:58:20Z DRILL-5136: Server unable to prepare some non SELECT queries Currently, the server makes every incoming queries a limit 0 query during server prepare. This is causing some non select queries to fail. We need to add an additional check to determine if the query is a SELECT query before wrapping it with LIMIT 0. --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] drill issue #602: Improve Drill C++ connector
Github user superbstreak commented on the issue: https://github.com/apache/drill/pull/602 **Mac Environment:** OS: OSX 10.8.5 COMPILER=xcode5_1 **Libraries:** Zookeeper: 3.4.6 (patched) boost: 1.57.0 protobuf: 2.5.0rc1 **For libc++:** .../contrib/native/client/CMakeLists.txt: - Add: set(CMAKE_CXX_FLAGS "-stdlib=libc++") before if(CMAKE_COMPILER_IS_GNUCXX) - Change to set(CMAKE_EXE_LINKER_FLAGS "-stdlib=libc++ -lrt -lpthread") - Change to set(CMAKE_CXX_FLAGS "-fPIC -stdlib=libc++") **And the minimal code changes required for zookeeper to work:** #ifndef htonll <- Add this int64_t htonll(int64_t v); #endif < Add this --- 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 feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---