[GitHub] geode-examples pull request #14: GEODE-3440: Add example for function execut...
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-examples/pull/14 GEODE-3440: Add example for function execution. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-examples feature/GEODE-3440 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-examples/pull/14.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 #14 commit a809fc829879b3df3d87ffa9ae99ed883267ef53 Author: Sarge <mdo...@pivotal.io> Date: 2017-08-14T23:40:27Z GEODE-3440: Add example for function execution. --- 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] geode-examples pull request #13: GEODE-3428: Add example of putting multiple...
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-examples/pull/13 GEODE-3428: Add example of putting multiple values all at once. Add an example of `Region.putAll()`. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-examples feature/GEODE-3428 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-examples/pull/13.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 #13 commit 50852add495930cc7d310efff191d72fda35f5c7 Author: Sarge <mdo...@pivotal.io> Date: 2017-08-11T18:50:53Z GEODE-3428: Add example of putting multiple values all at once. --- 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] geode-examples pull request #12: Add OQL example
Github user PivotalSarge closed the pull request at: https://github.com/apache/geode-examples/pull/12 --- 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] geode-examples pull request #12: Add OQL example
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-examples/pull/12 Add OQL example Perry Birch and I paired on an example for OQL. To prepare for that, I also created an example for Region.putAll(). You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-examples develop Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-examples/pull/12.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 #12 commit 98df7e42d0a4e390fd9114461af04a5cf9f8982e Author: Sarge <mdo...@pivotal.io> Date: 2017-08-04T22:42:07Z Add example for Region.putAll(). commit 20a8c44a8a8068cb4abc2caca5fd278074cf8167 Author: Sarge <mdo...@pivotal.io> Date: 2017-08-04T22:46:28Z Fix sample gfsh command. commit ac81624d7a809387068b0e0a71c0854c14b477ac Author: Sarge <mdo...@pivotal.io> Date: 2017-08-07T21:18:54Z Replace foreach loop of put() with putAll(). commit 98d2325964145518f971793a67ea0c9a57509024 Author: Sarge <mdo...@pivotal.io> Date: 2017-08-07T22:52:15Z Add example for OQL. commit b151f6285f7d3653d407cfb20ff85c22d6ee59af Author: Sarge <mdo...@pivotal.io> Date: 2017-08-08T18:44:46Z Remove unused java.util.Random. commit 06e82508d57050ec3760e6a0cb66164a2d330b95 Author: Sarge <mdo...@pivotal.io> Date: 2017-08-08T18:45:34Z Add Perry's changes. commit 43dd33833c306de21aaaf4cfa723a5f0408bc573 Author: Sarge <mdo...@pivotal.io> Date: 2017-08-10T20:27:29Z Fix putAll example test. commit eb747a6d359fb2853894db7b94e289c3c58be358 Author: Sarge <mdo...@pivotal.io> Date: 2017-08-10T22:16:29Z Reduce the OQL example down to the consecutive queries example. --- 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] geode-native issue #74: GEODE-2713: Wrap result collector lock in shared_ptr...
Github user PivotalSarge commented on the issue: https://github.com/apache/geode-native/pull/74 To quote Montgomery Scott, "I did what I could, Captain!" > On 4 Apr, 2017, at 11:40, Ernie Burghardt <notificati...@github.com> wrote: > > @PivotalSarge <https://github.com/PivotalSarge> need to clean up this PR, should not have all of the docs commits included, TIA > > â > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub <https://github.com/apache/geode-native/pull/74#issuecomment-291593880>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ATlz4qbKjC__AlGa5bSDX0mzozBlrF1mks5rso6pgaJpZM4Mosqr>. > --- 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] geode-native pull request #82: GEODE-2736: Fixed orphaned worker threads
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/82#discussion_r109194938 --- Diff: src/cppcache/integration-test/testThinClientPoolExecuteFunctionThrowsException.cpp --- @@ -0,0 +1,362 @@ +/* + * 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. + */ + +#define ROOT_NAME "testThinClientPoolExecuteFunctionThrowsException" + +#include "fw_dunit.hpp" +#include "ThinClientHelper.hpp" +#include "testobject/VariousPdxTypes.hpp" + +#include +#include + +using namespace PdxTests; +/* This is to test +1- funtion execution on pool + */ + +#define CLIENT1 s1p1 +#define LOCATOR1 s2p1 +#define SERVER s2p2 + +bool isLocalServer = false; +bool isLocator = false; +bool isPoolWithEndpoint = false; + +const char* locHostPort = +CacheHelper::getLocatorHostPort(isLocator, isLocalServer, 1); +const char* poolRegNames[] = {"partition_region", "PoolRegion2"}; + +const char* serverGroup = "ServerGroup1"; + +char* getFuncIName = (char*)"MultiGetFunctionI"; +char* putFuncIName = (char*)"MultiPutFunctionI"; +char* getFuncName = (char*)"MultiGetFunction"; +char* putFuncName = (char*)"MultiPutFunction"; +char* rjFuncName = (char*)"RegionOperationsFunction"; +char* exFuncName = (char*)"ExceptionHandlingFunction"; +char* exFuncNameSendException = (char*)"executeFunction_SendException"; +char* exFuncNamePdxType = (char*)"PdxFunctionTest"; +char* FEOnRegionPrSHOP = (char*)"FEOnRegionPrSHOP"; +char* FEOnRegionPrSHOP_OptimizeForWrite = +(char*)"FEOnRegionPrSHOP_OptimizeForWrite"; +char* FETimeOut = (char*)"FunctionExecutionTimeOut"; + +#define verifyGetResults() \ +bool found = false; \ +for (int j = 0; j < 34; j++) { \ + if (j % 2 == 0) continue; \ + sprintf(buf, "VALUE--%d", j); \ + if (strcmp(buf, dynCast(resultList->operator[](i))\ + ->asChar()) == 0) { \ +LOGINFO( \ + "buf = %s " \ + "dynCast(resultList->operator[](i))->asChar() " \ + "= %s ",\ + buf,\ + dynCast(resultList->operator[](i))->asChar()); \ + found = true; \ + break; \ + } \ +} \ +ASSERT(found, "this returned value is invalid"); + +#define verifyGetKeyResults() \ +bool found = false;
[GitHub] geode-native pull request #81: GEODE-2513 Unbranding docs: QuickStart exampl...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/81#discussion_r108795285 --- Diff: BUILDING.md --- @@ -39,6 +39,9 @@ Building requires access to an installation of Geode. By default the value of `G ## Installing By default a system-specific location is used by CMake as the destination of the `install` target, e.g., `/usr/local` on UNIX system. To explicitly specify the location in which the Native Client will be installed, add `-DCMAKE_INSTALL_PREFIX=/path/to/installation/destination` to the _initial_ `cmake` execution command before `../src`. +**Note:** For consistent results, avoid using the "~" (tilde) abbreviation when specifying paths on the CMake command line. --- End diff -- Good catch. --- 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] geode-native pull request #77: GEODE-2513 Unbrand docs section Configuring t...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/77#discussion_r108702132 --- Diff: docs/geode-native-docs/client-cache/application-plugins.html.md.erb --- @@ -262,7 +261,7 @@ void removeListener(RegionPtr& region) ## Considerations for Implementing Callbacks -Keep your callback implementations lightweight and prevent situations that might cause them to hang. For example, do not perform distribution operations or disconnects inside events. +Keep your callback implementations lightweight and prevent situations that might cause them to hang. For example, do not perform distribution operations or disconnects inside event handlers. --- End diff -- More accurate. --- 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] geode-native pull request #77: GEODE-2513 Unbrand docs section Configuring t...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/77#discussion_r108701675 --- Diff: docs/geode-native-docs/cache-init-file/chapter-overview.html.md.erb --- @@ -33,26 +33,23 @@ The initialization file can have any name, but is generally referred to as `cach The contents of a declarative XML file correspond to APIs declared in the `Cache.hpp` and `Region.hpp` header files. The cache initialization file allows you to accomplish declaratively many of the cache management activities that you can program through the API. -- The contents of the cache initialization file must conform to the XML definition in _product-dir_/dtd/gfcpp-cache8000.dtd. The DTD file identifies the valid element tags that may be present in your XML file, the attributes that correspond to each element, and the valid values for the elements and attributes. -- The name of the declarative XML file is specified when establishing a connection to the distributed system. You can define it by setting the `cache-xml-file` configuration attribute in the `geode.properties` file for the native client. For details about the `geode.properties` file, see [Setting System and Cache Properties](../setting-properties/chapter-overview.html). +- The contents of the cache initialization file must conform to the XML definition in [http://geode.apache.org/schema/cache/cache-1.0.xsd](http://geode.apache.org/schema/cache/cache-1.0.xsd). This file identifies the valid element tags that may be present in your XML file, the attributes that correspond to each element, and the valid values for the elements and attributes. +- The name of the declarative XML file is specified when establishing a connection to the distributed system. You can define it by setting the `cache-xml-file` configuration attribute in the `geode.properties` file for the client. For details about the `geode.properties` file, see [Setting System and Cache Properties](../setting-properties/chapter-overview.html). # Example cache.xml File An example `cache.xml` file shows cache and region initialization for a client, presenting a subset of the possible data configurations. -Specific information about cache and region attributes is in [Region Attributes](../client-cache/region-attributes.html). Also check the API documentation for `Cache` and `RegionAttributes` at [http://gemfire-apis.docs.pivotal.io](http://gemfire-apis.docs.pivotal.io). +Specific information about cache and region attributes is in [Region Attributes](../client-cache/region-attributes.html). -For information on using a cache with a server pool, see [Using Connection Pools](../connection-pools/connection-pools.html). The example below shows a `cache.xml` file that creates two regions. - -- Region `region1` is defined with a full set of region attributes and application plug-ins. The region's entries have `RegionTimeToLive` and `RegionIdleTimeout` expiration attributes set, as detailed in [Specifying Expiration Attributes](../client-cache/expiration-attributes.html). -- Region `region2` uses mostly default values. +For information on using a cache with a server pool, see [Using Connection Pools](../connection-pools/connection-pools.html). The example below shows a `cache.xml` file that creates a region. ``` pre http://www.example.com/dtd/gfcpp-cache8000.dtd;> +"-//Example Systems, Inc.//Example Declarative Caching 1.0//EN" +"http://geode.apache.org/schema/cache/cache-1.0.xsd;> --- End diff -- Nicely done. --- 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] geode-native pull request #75: GEODE-2513 Unbrand .NET API docs section
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/75#discussion_r108524165 --- Diff: docs/geode-native-book/master_middleman/source/subnavs/geode-nc-nav.erb --- @@ -439,7 +439,7 @@ limitations under the License. -Serialize with the GemFire IGFSerializable Interface +Serialize with the Geode IGFSerializable Interface --- End diff -- I believe `IGFSerializable` has been replaced with `IGeodeSerializable`. --- 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] geode-native pull request #75: GEODE-2513 Unbrand .NET API docs section
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/75#discussion_r108500455 --- Diff: docs/geode-native-book/master_middleman/source/subnavs/geode-nc-nav.erb --- @@ -473,7 +473,7 @@ limitations under the License. Resolving the Error -Using GemStone.GemFire.Cache.dll As a Private Assembly +Using Apache.Geode.Client.dll As a Private Assembly --- End diff -- I don't see `Apache.Geode.Client.dll`, only `Apache.Geode.dll`, after a Windows installation. --- 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] geode-native pull request #75: GEODE-2513 Unbrand .NET API docs section
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/75#discussion_r108496561 --- Diff: docs/geode-native-docs/dotnet-caching-api/implementing-igfserializable.html.md.erb --- @@ -31,21 +31,21 @@ Examples follow the procedure. void ToData(DataOutput output) ``` -The `ToData` function is responsible for copying all of the data fields for the object to the object stream. The `DataOutput` class represents the output stream and provides methods for writing the primitives in a network byte order. For details, see the API documentation for `DataOutput` at [http://gemfire-apis.docs.pivotal.io](http://gemfire-apis.docs.pivotal.io). +The `ToData` function is responsible for copying all of the data fields for the object to the object stream. The `DataOutput` class represents the output stream and provides methods for writing the primitives in a network byte order. 2. Implement the `FromData` function that consumes a data input stream and repopulates the data fields for the object: ``` pre void fromData (DataInput& input) ``` -The `DataInput` class represents the input stream and provides methods for reading input elements. The `FromData` function must read the elements of the input stream in the same order that they were written by `ToData`. For more about this, see the API documentation for `DataInput` at [http://gemfire-apis.docs.pivotal.io](http://gemfire-apis.docs.pivotal.io). +The `DataInput` class represents the input stream and provides methods for reading input elements. The `FromData` function must read the elements of the input stream in the same order that they were written by `ToData`. 3. Implement the `ClassId` function to return an integer which is unique for your class (in the set of all of your user-defined classes). ## Simple BankAccount Class -This example shows a simple class, `BankAccount`, that encapsulates two `ints`: `customerId` and `accountId`: +This example shows a simple class, `BankAccount`, that encapsulates the two `ints` `customerId` and `accountId`: --- End diff -- Maybe a comma between `ints` and `customerId`? --- 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] geode-native pull request #74: GEODE-2713: Wrap result collector lock in sha...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/74#discussion_r107995262 --- Diff: src/cppcache/src/ThinClientRegion.hpp --- @@ -416,14 +416,21 @@ class ChunkedFunctionExecutionResponse : public TcrChunkedResult { public: inline ChunkedFunctionExecutionResponse( - TcrMessage& msg, bool getResult, ResultCollectorPtr rc, - ACE_Recursive_Thread_Mutex* resultCollectorLock = NULL) + TcrMessage& msg, bool getResult, ResultCollectorPtr rc) : TcrChunkedResult(), m_msg(msg), m_getResult(getResult), -m_rc(rc), -m_resultCollectorLock(resultCollectorLock) {} - +m_rc(rc) {} + +inline ChunkedFunctionExecutionResponse( --- End diff -- There are two constructors now because the previous solitary constructor had the last parameter defaulted to `NULL`. Now that it's a smart pointer, I split the constructor into two pieces, one with the lock and one without. --- 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] geode-native pull request #74: GEODE-2713: Wrap result collector lock in sha...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/74#discussion_r107993045 --- Diff: src/cppcache/src/ThinClientRegion.hpp --- @@ -416,14 +416,21 @@ class ChunkedFunctionExecutionResponse : public TcrChunkedResult { public: inline ChunkedFunctionExecutionResponse( - TcrMessage& msg, bool getResult, ResultCollectorPtr rc, - ACE_Recursive_Thread_Mutex* resultCollectorLock = NULL) + TcrMessage& msg, bool getResult, ResultCollectorPtr rc) : TcrChunkedResult(), m_msg(msg), m_getResult(getResult), -m_rc(rc), -m_resultCollectorLock(resultCollectorLock) {} - +m_rc(rc) {} + +inline ChunkedFunctionExecutionResponse( --- End diff -- This code is exercised during function execution (ExecutionImpl.cpp and ThinClientPoolDM.hpp) and thus is covered by the function execution tests. --- 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] geode-native pull request #74: GEODE-2713: Wrap result collector lock in sha...
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/74 GEODE-2713: Wrap result collector lock in shared_ptr. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2713 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/74.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 #74 commit 422b9dbe1e59621019c971b111322a9b49b431d7 Author: Sarge <mdo...@pivotal.io> Date: 2017-03-24T18:06:29Z GEODE-2713: Wrap result collector lock in shared_ptr. --- 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] geode-native pull request #73: GEODE-2513 Unbrand C++ API docs: Getting Star...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/73#discussion_r107939092 --- Diff: docs/geode-native-book/master_middleman/source/subnavs/geode-nc-nav.erb --- @@ -20,26 +20,26 @@ limitations under the License. -Pivotal GemFire Native Client 9.0 Documentation +Apache Geode Client Documentation - -Supported Configurations and System Requirements - + + System Configurations + -Getting Started with a Native Client +Getting Started with the Client -About the Native Client +About the Client -Installing the Native Client +Installing the Geode Client --- End diff -- Elsewhere it's just "Client", not "Geode Client". --- 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] geode-native pull request #63: GEODE-2657 - Fixes and tests Function Executi...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/63#discussion_r106922752 --- Diff: src/clicache/src/CacheFactory.cpp --- @@ -84,6 +85,12 @@ namespace Apache //TODO::split SafeConvertClassGeneric::SetAppDomainEnabled(appDomainEnable); + if (appDomainEnable) + { +// Register managed AppDomain context with unmanaged. +apache::geode::client::createAppDomainContext = ::Geode::Client::createAppDomainContext; --- End diff -- Does moving the registration of the managed app domain context to cache factory from distributed system make it available sooner? --- 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] geode-native pull request #59: GEODE-2513 Unbrand Function Execution section...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/59#discussion_r106483611 --- Diff: docs/geode-native-docs/function-execution/handling-function-results.html.md.erb --- @@ -19,7 +19,7 @@ See the License for the specific language governing permissions and limitations under the License. --> -Geode provides a default result collector. If you need special results handling, code a custom `ResultsCollector` implementation to replace the provided default. Use the `Execution::withCollector` method to define your custom collector. +There is a default result collector. If you need special results handling, code a custom `ResultsCollector` implementation to replace the provided default. Use the `Execution::withCollector` method to define your custom collector. --- End diff -- Would it be better to make the default results collector more active, e.g., "The default results collector provides the following functionality..."? --- 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] geode-native pull request #50: GEODE-2636: Switch to CMake variable for libr...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/50#discussion_r105266335 --- Diff: src/quickstart/cpp/PdxSerializer.cpp --- @@ -165,15 +165,15 @@ int main(int argc, char** argv) { LOGINFO("Registered Person Query Objects"); // Populate the Region with some Person objects. -Person* p1 = new Person("John", 1 /*ID*/, 23 /*age*/); +Person* p1 = new Person((char *)"John", 1 /*ID*/, 23 /*age*/); --- End diff -- Those methods pass a string literal to a method whose first parameter is char *. An alternative would be to modify the type of the parameter but that was a more invasive change when the goal was to reduce the clutter in the log that got in the way of determining the root cause. --- 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] geode-native pull request #50: GEODE-2636: Switch to CMake variable for libr...
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/50 GEODE-2636: Switch to CMake variable for library name. - Parameterize the name of the library for which the quickstarts look. - Fix warnings that obfuscate when the quickstarts can not find a library. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2636 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/50.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 #50 commit e897dd59a4f03eb22483bb41b8db38bb84686132 Author: Sarge <mdo...@pivotal.io> Date: 2017-03-08T22:28:52Z GEODE-2636: Switch to CMake variable for library name. --- 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] geode-native pull request #48: GEODE-2578: Remove 64 KiB limit on query stri...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/48#discussion_r104836958 --- Diff: src/cppcache/include/geode/DataOutput.hpp --- @@ -378,9 +378,8 @@ class CPPCACHE_EXPORT DataOutput { */ inline void writeFullUTF(const char* value, uint32_t length = 0) { --- End diff -- I agree and tried to move it to TcrMessage. However, its implementation depends on private members of DataOutput. --- 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] geode-native pull request #48: GEODE-2578: Remove 64 KiB limit on query stri...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/48#discussion_r104769132 --- Diff: src/cppcache/test/DataOutputTest.cpp --- @@ -305,15 +305,9 @@ TEST_F(DataOutputTest, TestCursorAdvance) { "001B596F7520686164206D65206174206D65617420746F726E61646F2E", dataOutput.getByteArray()); - EXPECT_EQ((2 + 27), dataOutput.getBufferLength()); - - // buffers are pre-allocated 8k and have 2 bytes to hold the data length - EXPECT_EQ(((8 * 1024) - (2 + 27)), dataOutput.getRemainingBufferLength()); --- End diff -- getRemainingBufferLength() is the call that exhibited less-than-obvious behavior. In particular, it appears to be affected by some global state. Its return value was not predictable enough to be tested in that manner. --- 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] geode-native pull request #48: GEODE-2578: Remove 64 KiB limit on query stri...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/48#discussion_r104768886 --- Diff: src/cppcache/include/geode/DataOutput.hpp --- @@ -378,9 +378,8 @@ class CPPCACHE_EXPORT DataOutput { */ inline void writeFullUTF(const char* value, uint32_t length = 0) { if (value != NULL) { - int32_t len = getEncodedLength(value, length); - uint16_t encodedLen = static_cast(len > 0x ? 0x : len); --- End diff -- If the query string happened to be longer that 0x, it would be artificially truncated which often led to errors on the server side. --- 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] geode-native pull request #48: GEODE-2578: Remove 64 KiB limit on query stri...
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/48 GEODE-2578: Remove 64 KiB limit on query strings. - Remove artificial cap of 65535 for query string length by using 32 bits for the length of query strings in DataOutput::writeFullUTF(). - Rename parameter to TcrMessage::writeStringPart() whose name is misleading due to copy-and-paste. - Add three unit tests for query-related messages: * testConstructorEXECUTECQ_MSG_TYPE * testConstructorWithGinormousQueryEXECUTECQ_MSG_TYPE * testConstructorEXECUTECQ_WITH_IR_MSG_TYPE - Fix the cursor advance tests to not depend on specific values for buffer length and not to test remaining buffer length. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2578 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/48.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 #48 commit 128ab410d3aaae69186c0a41ed3a1310ef982382 Author: Sarge <mdo...@pivotal.io> Date: 2017-03-07T19:26:53Z GEODE-2578: Remove 64 KiB limit on query strings. - Remove artificial cap of 65535 for query string length by using 32 bits for the length of query strings in DataOutput::writeFullUTF(). - Rename parameter to TcrMessage::writeStringPart() whose name is misleading due to copy-and-paste. - Add three unit tests for query-related messages: * testConstructorEXECUTECQ_MSG_TYPE * testConstructorWithGinormousQueryEXECUTECQ_MSG_TYPE * testConstructorEXECUTECQ_WITH_IR_MSG_TYPE - Fix the cursor advance tests to not depend on specific values for buffer length and not to test remaining buffer length. --- 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] geode-native issue #44: GEODE-2578: Remove 64 KiB limit on query strings.
Github user PivotalSarge commented on the issue: https://github.com/apache/geode-native/pull/44 Roger that. Expect a new PR forthwith... Sarge > On 7 Mar, 2017, at 08:10, Jacob Barrett <notificati...@github.com> wrote: > > @PivotalSarge <https://github.com/PivotalSarge> can you rebase your branch off of develop. We backed out all those C++11 changes and now they are showing up in your pull. > > â > You are receiving this because you were mentioned. > Reply to this email directly, view it on GitHub <https://github.com/apache/geode-native/pull/44#issuecomment-284767886>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ATlz4jShpxAMJQ6goU2ocGZqUWWhijVQks5rjYGPgaJpZM4MRo4O>. > --- 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] geode-native pull request #44: GEODE-2578: Remove 64 KiB limit on query stri...
Github user PivotalSarge closed the pull request at: https://github.com/apache/geode-native/pull/44 --- 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] geode-native issue #44: GEODE-2578: Remove 64 KiB limit on query strings.
Github user PivotalSarge commented on the issue: https://github.com/apache/geode-native/pull/44 The unit tests failed on Linux but not other platforms. It turns out that DataOutput has some not-very-obvious (and apparently platform-specific) behavior around buffer management that results in the return values from getBufferLength() and getRemainingBufferLength() depending on the behavior of other unit tests that may precede them. Hence, the test of ginormous query strings was causing the return values of those methods to differ from what would be returned if that test was *not* run. So I reworked how the DataOutput cursor advancement unit tests work and created GEODE-2598 to understand and test how DataOutput's buffer management works. --- 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] geode-native pull request #44: GEODE-2578: Remove 64 KiB limit on query stri...
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/44 GEODE-2578: Remove 64 KiB limit on query strings. - Remove artificial cap of 65535 for query string length by using 32 bits for the length of query strings in DataOutput::writeFullUTF(). - Rename parameter to TcrMessage::writeStringPart() whose name is misleading due to copy-and-paste. - Add three unit tests for query-related messages: * testConstructorEXECUTECQ_MSG_TYPE * testConstructorWithGinormousQueryEXECUTECQ_MSG_TYPE * testConstructorEXECUTECQ_WITH_IR_MSG_TYPE You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2578 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/44.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 #44 commit 542c6349c7194558f463732a96a0e32a216a17a3 Author: Sarge <mdo...@pivotal.io> Date: 2017-03-02T22:45:37Z GEODE-2578: Remove 64 KiB limit on query strings. - Remove artificial cap of 65535 for query string length by using 32 bits for the length of query strings in DataOutput::writeFullUTF(). - Rename parameter to TcrMessage::writeStringPart() whose name is misleading due to copy-and-paste. - Add three unit tests for query-related messages: * testConstructorEXECUTECQ_MSG_TYPE * testConstructorWithGinormousQueryEXECUTECQ_MSG_TYPE * testConstructorEXECUTECQ_WITH_IR_MSG_TYPE --- 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] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/36#discussion_r103281152 --- Diff: src/tests/cpp/security/Security.cpp --- @@ -41,16 +41,18 @@ #include "security/CredentialGenerator.hpp" -namespace FwkSecurity { +#include +#include + +namespace apache { +namespace geode { +namespace client { +namespace testframework { +namespace security { --- End diff -- Could these constants be moved into an anonymous namespace? --- 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] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/36#discussion_r103280043 --- Diff: src/cppcache/src/LRUEntriesMap.cpp --- @@ -297,7 +295,7 @@ GfErrType LRUEntriesMap::put(const CacheableKeyPtr& key, } } // SpinLock& lock = segmentRPtr->getSpinLock(); -// SpinLockGuard mapGuard( lock ); +// std::lock_guard mapGuard( lock ); --- End diff -- Remove commented out code? --- 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] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/36#discussion_r103280113 --- Diff: src/cppcache/src/LRUEntriesMap.cpp --- @@ -20,6 +20,9 @@ #include "MapSegment.hpp" #include "CacheImpl.hpp" +#include +#include "util/concurrent/spinlock_mutex.hpp" --- End diff -- Similar to a previous comment from @dgkimura. --- 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] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/36#discussion_r103279753 --- Diff: src/cppcache/src/MapSegment.hpp --- @@ -164,9 +166,9 @@ class CPPCACHE_EXPORT MapSegment { m_entryFactory->newMapEntry(key, newEntry); newEntry->setValueI(newValue); if (m_concurrencyChecksEnabled) { - if (versionTag != NULLPTR && versionTag.ptr() != NULL) { + if (versionTag != NULLPTR && versionTag.ptr() != nullptr) { --- End diff -- It looks like `nullptr` is being used, not `NULLPTR`, no? --- 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] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/36#discussion_r103279475 --- Diff: src/cppcache/src/MapSegment.cpp --- @@ -316,16 +320,16 @@ GfErrType MapSegment::remove(const CacheableKeyPtr& key, CacheablePtr& oldValue, MapEntryImplPtr& me, int updateCount, VersionTagPtr versionTag, bool afterRemote, bool& isEntryFound) { - // _GF_GUARD_SEGMENT; + // std::lock_guard lk(m_spinlock); --- End diff -- Since the code is commented out, should it just be deleted? --- 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] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/36#discussion_r103278690 --- Diff: src/cppcache/src/LRUList.cpp --- @@ -15,12 +15,18 @@ * limitations under the License. */ #include "LRUList.hpp" -#include "SpinLock.hpp" +#include "util/concurrent/spinlock_mutex.hpp" -using namespace apache::geode::client; +#include -#define LOCK_HEAD SpinLockGuard headLockGuard(m_headLock) -#define LOCK_TAIL SpinLockGuard tailLockGuard(m_tailLock) --- End diff -- Good to see the removal of more macros. --- 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] geode-native pull request #36: GEODE-2494: Replace SpinLock with spinlock_mu...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/36#discussion_r103279163 --- Diff: src/cppcache/src/LRUList.hpp --- @@ -20,32 +20,32 @@ * limitations under the License. */ +#include + #include #include -#include "SpinLock.hpp" + +#include "util/concurrent/spinlock_mutex.hpp" namespace apache { namespace geode { namespace client { + // Bit mask for recently used -#define RECENTLY_USED_BITS 1ul +#define RECENTLY_USED_BITS 1u // Bit mask for evicted -#define EVICTED_BITS 2ul +#define EVICTED_BITS 2u --- End diff -- Could these be const variables in an anonymous namespace instead? --- 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] geode-native pull request #37: GEODE-2531: Replace HostAsm::atomic with std:...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/37#discussion_r103255168 --- Diff: src/cppcache/include/geode/SharedBase.hpp --- @@ -56,11 +57,12 @@ class CPPCACHE_EXPORT SharedBase { protected: inline SharedBase(bool noInit) {} + inline SharedBase(const SharedBase&) {} virtual ~SharedBase() {} private: - mutable volatile int32_t m_refCount; + std::atomic m_refCount; --- End diff -- Is the mutable unnecessary? Or does std::atomic have const operators that can be used to mutate the reference count? --- 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] geode-native pull request #37: GEODE-2531: Replace HostAsm::atomic with std:...
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/37#discussion_r103254380 --- Diff: src/cppcache/include/geode/CacheStatistics.hpp --- @@ -102,8 +102,8 @@ class CPPCACHE_EXPORT CacheStatistics : public SharedBase { virtual void setLastAccessedTime(uint32_t lat); virtual void setLastModifiedTime(uint32_t lmt); - volatile uint32_t m_lastAccessTime; - volatile uint32_t m_lastModifiedTime; + std::atomic m_lastAccessTime; + std::atomic m_lastModifiedTime; --- End diff -- std::atomic is pretty cool. It looks like the performance impact is negligible, right? --- 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] geode-native issue #39: Add rat checks to travis-ci build
Github user PivotalSarge commented on the issue: https://github.com/apache/geode-native/pull/39 Where is .ratignore? I don't see it. --- 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] geode-native pull request #29: GEODE-2480: Correct the version of CMake.
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/29 GEODE-2480: Correct the version of CMake. - Modify BUILDING.md to correct the typo in the version of CMake. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2480 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/29.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 #29 commit 3652129ebef39ba629a7233ed9dcb8e33713ee06 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-23T18:52:41Z GEODE-2480: Correct the version of CMake. --- 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] geode-native pull request #25: GEODE-2382: Replace gemstone with geode.
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/25#discussion_r102769835 --- Diff: src/xsds/gfcpp-cache-9.0.xsd --- @@ -33,7 +33,7 @@ limitations under the License. version="9.0"> The contents of a declarative XML file correspond to APIs found in the -com.gemstone.gemfire.cache and com.gemstone.gemfire.cache.client +org.apache.geode.cache and org.apache.geode.cache.client --- End diff -- I got those packages names off a checkout of the develop branch of geode yesterday so I'm pretty confident they're right. --- 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] geode-native issue #23: GEODE-2478: Replace gf with geode.
Github user PivotalSarge commented on the issue: https://github.com/apache/geode-native/pull/23 Good idea. I think you're right about redundancies. I'll look into that. Sarge > On 22 Feb, 2017, at 10:48, Jacob Barrett <notificati...@github.com> wrote: > > @pivotal-jbarrett approved this pull request. > > I would suggest opening a ticket with any cleanup ideas you may have seen when going through all these headers. Seems like geode_defs and geode_base and and whatever generically named includes we have might be fairly redundant and ripe for cleanup or reorganization. > > Otherwise this looks good. > > â > You are receiving this because you authored the thread. > Reply to this email directly, view it on GitHub <https://github.com/apache/geode-native/pull/23#pullrequestreview-23299880>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ATlz4uDEUOa4K1RgRpgEMG-urzHriHQGks5rfILmgaJpZM4MI5mz>. > --- 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] geode-native pull request #23: GEODE-2478: Replace gf with geode.
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/23 GEODE-2478: Replace gf with geode. - Rename directories and files with gf into their name to instead use geode and update all references thereto. - Ensure formatting style guide compliance. - Elide duplicate GEODE_ from include guards. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2478 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/23.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 #23 commit a58b1440b76a3d693e720ce72b6cd18a26fbc8c0 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-22T03:41:10Z GEODE-2478: Replace gf with geode. - Rename directories and files with gf into their name to instead use geode and update all references thereto. - Ensure formatting style guide compliance. - Elide duplicate GEODE_ from include guards. --- 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] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101796694 --- Diff: src/CMakeLists.txt --- @@ -224,7 +222,7 @@ add_subdirectory(cppcache) add_subdirectory(cryptoimpl) add_subdirectory(dhimpl) add_subdirectory(sqliteimpl) -add_subdirectory(gfcpp) +add_subdirectory(getversion) --- End diff -- I'm totally cool with deleting it. I didn't because it's been part of the install; some users may rely on it to determine version? --- 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] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode-native/pull/13#discussion_r101796329 --- Diff: src/cppcache/include/geode/AttributesFactory.hpp --- @@ -20,7 +20,7 @@ * limitations under the License. */ -#include "gfcpp_globals.hpp" +#include "geode_globals.hpp" #include "gf_types.hpp" --- End diff -- That's a separate JIRA ticket and hence will be a separate (upcoming) PR. --- 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] geode-native pull request #13: GEODE-2476: Replace gfcpp with geode.
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/13 GEODE-2476: Replace gfcpp with geode. - Rename directories and files with gfcpp into their name to instead use geode and update all references thereto. - Rename the gfcpp executable to apache-geode-getversion and modify it to print the version even in the absence of command-line arguments. - Rename gfcpp.properties to geode.properties and update all references thereto. - Ensure formatting style guide compliance. - Re-applying fixes for Windows compilation errors. - Fix logic for using clang tidy auto-fix. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2476 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/13.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 #13 commit 56e21e5e89bed28416237c6d752d60f9d894e9b7 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-14T18:31:51Z GEODE-2476: Replace gfcpp with geode. - Rename directories and files with gfcpp into their name to instead use geode and update all references thereto. - Rename the gfcpp executable to apache-geode-getversion and modify it to print the version even in the absence of command-line arguments. - Rename gfcpp.properties to geode.properties and update all references thereto. - Ensure formatting style guide compliance. - Re-applying fixes for Windows compilation errors. - Fix logic for using clang tidy auto-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] geode-native pull request #10: Fix build on Windows.
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/10 Fix build on Windows. There was a lingering reference to one of the example files, ComplexNumber.cs, in the CLI tests. Restoring that file to a different location was insufficient because the CMake files also had references to it. Further research showed that the test was omitted anyway so I just removed all the offenders. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2462 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/10.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 #10 commit f1b432ee915341096295e39d1fcc8ba35449c281 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-14T18:43:52Z GEODE-2462: Restore example file used by the unit tests. commit a9b68fd7b05815e794d8ada31c1d2587985eef33 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-14T20:09:30Z GEODE-2462: Remove examples file used by unit tests. --- 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] geode-native issue #2: GEODE-2423: Remove unused keystore files and rename t...
Github user PivotalSarge commented on the issue: https://github.com/apache/geode-native/pull/2 LGTM --- 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] geode-native issue #7: Feature/geode 2467
Github user PivotalSarge commented on the issue: https://github.com/apache/geode-native/pull/7 LGTM --- 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] geode-native issue #9: [GEODE-2408] Refactor CacheableDate with C++11 standa...
Github user PivotalSarge commented on the issue: https://github.com/apache/geode-native/pull/9 LGTM --- 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] geode-native pull request #8: GEODE-2376: Remove low-value comments from Thi...
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/8 GEODE-2376: Remove low-value comments from ThinClientPoolDM.cpp. - Remove the stack trace comment. - Remove commented-out code that already exists in the history of the file in version control. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2376 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/8.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 #8 commit 5aaec654a5503f89728373039c62c1c23c7d4259 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-13T18:54:51Z GEODE-2376: Remove low-value comments from ThinClientPoolDM.cpp. - Remove the stack trace comment. - Remove commented-out code that already exists in the history of the file in version control. --- 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] geode-native pull request #5: GEODE-2462: Remove examples directory.
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/5 GEODE-2462: Remove examples directory. Remove the examples directory as it does not contain any examples that are not already covered in src/quickstart. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2462 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/5.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 #5 commit 80235ace8dcf5b5d309f35b5c5ebba585a6f37c8 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-10T23:49:25Z GEODE-2462: Remove examples directory. --- 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] geode-native pull request #4: GEODE-2446: Remove cclient directory.
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode-native/pull/4 GEODE-2446: Remove cclient directory. Remove the cclient directory which contains obsolete code. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode-native feature/GEODE-2446 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode-native/pull/4.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 #4 commit 36ee0289840b6ebe4f8adbac0d3b9d04098903d1 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-10T23:25:59Z GEODE-2446: Remove cclient directory. --- 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] geode issue #387: GEODE-2411: Remove references to Gemfire from include guar...
Github user PivotalSarge commented on the issue: https://github.com/apache/geode/pull/387 This closes #387. --- 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] geode pull request #387: GEODE-2411: Remove references to Gemfire from inclu...
Github user PivotalSarge closed the pull request at: https://github.com/apache/geode/pull/387 --- 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] geode issue #389: GEODE-1434: Add ASF headers
Github user PivotalSarge commented on the issue: https://github.com/apache/geode/pull/389 I pulled this PR, built it on Windows, and ran the quick integration tests. This validates that the header addition has not broken the Visual Studio project files. --- 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] geode issue #391: GEODE-2422: Switch remaining GemFire strings to Geode.
Github user PivotalSarge commented on the issue: https://github.com/apache/geode/pull/391 Done. GEODE-2348 > On 6 Feb, 2017, at 14:29, Michael Martell <notificati...@github.com> wrote: > > Good catch that must not be used. So we should remove it. Please create a new ticket for that. > > â > You are receiving this because you commented. > Reply to this email directly, view it on GitHub <https://github.com/apache/geode/pull/391#issuecomment-277835333>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ATlz4ufwOp5iV_f4FjWW1flVqHllf3Utks5rZ57OgaJpZM4L3QYi>. > --- 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] geode pull request #394: GEODE-2437 Add travis-ci file
Github user PivotalSarge commented on a diff in the pull request: https://github.com/apache/geode/pull/394#discussion_r99872593 --- Diff: .travis.yml --- @@ -0,0 +1,43 @@ +# 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. + +language: cpp +sudo: required + +before_install: + - sudo apt-get -qq update + - sudo apt-get -y build-essential + - sudo apt-get install -y cmake + - sudo apt-get install -y doxygen + - sudo apt-get install -y git + - sudo apt-get install -y openjdk-8-jdk + - sudo apt-get install -y wget + - sudo apt-get install -y zlib1g-dev + +install: mkdir build && cd build && cmake ../src && cmake --build . -- -j 8 --- End diff -- I don't believe the unit tests require GEODE to run. They certainly shouldn't. --- 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] geode issue #387: GEODE-2411: Remove references to Gemfire from include guar...
Github user PivotalSarge commented on the issue: https://github.com/apache/geode/pull/387 They've been googlified. --- 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] geode pull request #387: GEODE-2411: Remove references to Gemfire from inclu...
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode/pull/387 GEODE-2411: Remove references to Gemfire from include guards In the process of replacing references to Gemfire with references to Apache Geode in the include guards, standardize the include guards and use #pragma once where applicable. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode feature/GEODE-2411 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/387.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 #387 commit 3fc6b70429c2a4e247c164d7ad86ea0c0177b0a0 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-01T21:00:14Z GEODE-2411: Replace Gemfire-related include guards with #pragma once. commit 435f6bcf3edbf5e45bb404dee57f322ed4851649 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-01T21:15:40Z GEODE-2411: Add missing #pragma once. commit d2d4db0373f94cf12a6ab9b5265a29d8bb71838c Author: Sarge <mdo...@pivotal.io> Date: 2017-02-02T21:55:23Z GEODE-2411: Replace Gemfire-related include guards with #pragma once. commit 17181d074aa9ca1e9b759b18a17d6020ad57c93f Author: Sarge <mdo...@pivotal.io> Date: 2017-02-02T23:31:49Z GEODE-2411: Replace Gemfire-related include guards with #pragma once. commit c36f963ad8613534ebff87de2425a5eaa7992cc7 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-03T16:38:31Z GEODE-2411: Replace Gemfire-related include guards with #pragma once. commit 2c990d1be22ba9fa4bc4d096b44a5c1a9d633dc8 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-03T17:13:12Z GEODE-2411: Replace Gemfire-related include guards with #pragma once. commit e05b3e1a643cb54828d20a768f8c9e96a6e47c63 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-03T19:01:07Z GEODE-2411: Replace Gemfire-related include guards with #pragma once. commit e051494fde181b431caf2501894f764820e02450 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-03T19:09:29Z GEODE-2411: Replace Gemfire-related include guards with #pragma once. commit 414a91e712149e417dd421fa6a8307b2e3c7c95d Author: Sarge <mdo...@pivotal.io> Date: 2017-02-03T21:28:03Z GEODE-2411: Fix collateral damage from scripting. commit e376e56d90a582482bd117a774b0085b127a4448 Author: Sarge <mdo...@pivotal.io> Date: 2017-02-03T23:38:20Z GEODE-2411: Add back include guards for Solaris. --- 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] geode pull request #376: GEODE-2344: Updated the ordinal to match 9.0.
Github user PivotalSarge closed the pull request at: https://github.com/apache/geode/pull/376 --- 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] geode issue #376: GEODE-2344: Updated the ordinal to match 9.0.
Github user PivotalSarge commented on the issue: https://github.com/apache/geode/pull/376 It's just an integer literal value for a static constant variable called ordinal. Are you think along the lines of the remove #define statements? Is your concern determining which version of the wire protocol is supported? --- 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] geode pull request #376: GEODE-2344: Updated the ordinal to match 9.0.
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode/pull/376 GEODE-2344: Updated the ordinal to match 9.0. - Switch to the correct constructor to avoid errors. - Remove unnecessary mechanism to modify version ordinal. - Remove commented-out code. - Account for extra bytes in the handshaking after 8.2. - Refactor accommodating UUID and weight during handshake. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode feature/GEODE-2344 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/376.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 #376 commit b1928d0e8ed7f4c02659887a7b9f2a207a14aa1f Author: Sarge <mdo...@pivotal.io> Date: 2017-01-25T21:12:06Z GEODE-2344: Updated the ordinal to match 9.0. - Switch to the correct constructor to avoid errors. - Remove unnecessary mechanism to modify version ordinal. - Remove commented-out code. - Account for extra bytes in the handshaking after 8.2. - Refactor accommodating UUID and weight during handshake. --- 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] geode issue #344: Fix the Native Client Build on Ubuntu
Github user PivotalSarge commented on the issue: https://github.com/apache/geode/pull/344 Elegant. Looks good to me. --- 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] geode pull request #341: GEODE-2306: Update native client BUILDING.md to ref...
GitHub user PivotalSarge reopened a pull request: https://github.com/apache/geode/pull/341 GEODE-2306: Update native client BUILDING.md to reflect changes for Geode - Replaced GEMFIRE_HOME with GEODE_ROOT. - Corrected build instructions. - Minor copy-editing. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode feature/GEODE-2306 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/341.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 #341 commit d18d7d7fdc41a61f5184a562b828b8b16617d3f0 Author: Sarge <mdo...@pivotal.io> Date: 2017-01-17T01:20:00Z GEODE-2306: Update the build instructions for GEMFIRE_HOME. commit 49b58e7d450e62753af5f964fa68f2f951de7709 Author: Sarge <mdo...@pivotal.io> Date: 2017-01-17T15:16:59Z GEODE-2306: Update the build instructions for GEODE_ROOT. commit 480e690f21185c566a4f687b711561b44f3161b4 Author: Sarge <mdo...@pivotal.io> Date: 2017-01-17T15:43:27Z GEODE-2306: Mark Doxygen as required and fix typo. --- 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] geode issue #341: GEODE-2306: Update native client BUILDING.md to reflect ch...
Github user PivotalSarge commented on the issue: https://github.com/apache/geode/pull/341 Mark Doxygen as required and fix typo. --- 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] geode pull request #341: GEODE-2306: Update native client BUILDING.md to ref...
Github user PivotalSarge closed the pull request at: https://github.com/apache/geode/pull/341 --- 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] geode pull request #341: GEODE-2306: Update native client BUILDING.md to ref...
GitHub user PivotalSarge opened a pull request: https://github.com/apache/geode/pull/341 GEODE-2306: Update native client BUILDING.md to reflect changes for Geode - Replaced GEMFIRE_HOME with GEODE_ROOT. - Corrected build instructions. - Minor copy-editing. You can merge this pull request into a Git repository by running: $ git pull https://github.com/PivotalSarge/geode feature/GEODE-2306 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/geode/pull/341.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 #341 commit d18d7d7fdc41a61f5184a562b828b8b16617d3f0 Author: Sarge <mdo...@pivotal.io> Date: 2017-01-17T01:20:00Z GEODE-2306: Update the build instructions for GEMFIRE_HOME. commit 49b58e7d450e62753af5f964fa68f2f951de7709 Author: Sarge <mdo...@pivotal.io> Date: 2017-01-17T15:16:59Z GEODE-2306: Update the build instructions for GEODE_ROOT. --- 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. ---