You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@geode.apache.org by dgkimura <gi...@git.apache.org> on 2017/03/31 19:12:22 UTC

[GitHub] geode-native pull request #83: GEODE-2691: Fix function execution attributes...

GitHub user dgkimura opened a pull request:

    https://github.com/apache/geode-native/pull/83

    GEODE-2691: Fix function execution attributes mismatch

    Fixes function execution invocation which previously passed in attributes that
    function execute would compare to the server's view of the registered function
    attributes. Since the previously passed in attributes were deprecated there is
    nothing to compare. Instead we should accept the attributes that the server
    returns.
    
    Testing: Function execution tests pass

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/dgkimura/geode-native feature/GEODE-2691

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/geode-native/pull/83.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 #83
    
----
commit c97ef8358c9a9b625316380c823eea029230c5ba
Author: David Kimura <dk...@pivotal.io>
Date:   2017-03-31T18:33:59Z

    GEODE-2691: Fix function execution attributes mismatch
    
    Fixes function execution invocation which previously passed in attributes that
    function execute would compare to the server's view of the registered function
    attributes. Since the previously passed in attributes were deprecated there is
    nothing to compare. Instead we should accept the attributes that the server
    returns.
    
    Testing: Function execution tests pass

----


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #83: GEODE-2691: Fix function execution attributes...

Posted by PivotalSarge <gi...@git.apache.org>.
Github user PivotalSarge commented on a diff in the pull request:

    https://github.com/apache/geode-native/pull/83#discussion_r109242439
  
    --- Diff: src/cppcache/src/ExecutionImpl.cpp ---
    @@ -100,33 +94,31 @@ ResultCollectorPtr ExecutionImpl::execute(const char* fn, uint32_t timeout,
       bool serverIsHA = false;
       bool serverOptimizeForWrite = false;
     
    -  if (verifyFuncArgs) {
    -    std::vector<int8_t>* attr = getFunctionAttributes(fn);
    -    {
    +  std::vector<int8_t>* attr = getFunctionAttributes(fn);
    +  {
    --- End diff --
    
    What is the purpose of this curly-brace block? Is there an RAII instance in there?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #83: GEODE-2691: Fix function execution attributes...

Posted by echobravopapa <gi...@git.apache.org>.
Github user echobravopapa commented on a diff in the pull request:

    https://github.com/apache/geode-native/pull/83#discussion_r109743478
  
    --- Diff: src/cppcache/src/ExecutionImpl.cpp ---
    @@ -100,33 +94,31 @@ ResultCollectorPtr ExecutionImpl::execute(const char* fn, uint32_t timeout,
       bool serverIsHA = false;
       bool serverOptimizeForWrite = false;
     
    -  if (verifyFuncArgs) {
    -    std::vector<int8_t>* attr = getFunctionAttributes(fn);
    -    {
    +  std::vector<int8_t>* attr = getFunctionAttributes(fn);
    +  {
    --- End diff --
    
    I take that back... @dgkimura what is with the curly braced block?


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #83: GEODE-2691: Fix function execution attributes...

Posted by dgkimura <gi...@git.apache.org>.
Github user dgkimura commented on a diff in the pull request:

    https://github.com/apache/geode-native/pull/83#discussion_r109744732
  
    --- Diff: src/cppcache/src/ExecutionImpl.cpp ---
    @@ -100,33 +94,31 @@ ResultCollectorPtr ExecutionImpl::execute(const char* fn, uint32_t timeout,
       bool serverIsHA = false;
       bool serverOptimizeForWrite = false;
     
    -  if (verifyFuncArgs) {
    -    std::vector<int8_t>* attr = getFunctionAttributes(fn);
    -    {
    +  std::vector<int8_t>* attr = getFunctionAttributes(fn);
    +  {
    --- End diff --
    
    The curly-brace block was not introduced as part of this change, so your guess is as good as mine. :)


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #83: GEODE-2691: Fix function execution attributes...

Posted by echobravopapa <gi...@git.apache.org>.
Github user echobravopapa commented on a diff in the pull request:

    https://github.com/apache/geode-native/pull/83#discussion_r109743229
  
    --- Diff: src/cppcache/src/ExecutionImpl.cpp ---
    @@ -100,33 +94,31 @@ ResultCollectorPtr ExecutionImpl::execute(const char* fn, uint32_t timeout,
       bool serverIsHA = false;
       bool serverOptimizeForWrite = false;
     
    -  if (verifyFuncArgs) {
    -    std::vector<int8_t>* attr = getFunctionAttributes(fn);
    -    {
    +  std::vector<int8_t>* attr = getFunctionAttributes(fn);
    +  {
    --- End diff --
    
    looks like it is an artifact from the previous if statement and can be removed


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native pull request #83: GEODE-2691: Fix function execution attributes...

Posted by dgkimura <gi...@git.apache.org>.
Github user dgkimura closed the pull request at:

    https://github.com/apache/geode-native/pull/83


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

[GitHub] geode-native issue #83: GEODE-2691: Fix function execution attributes mismat...

Posted by echobravopapa <gi...@git.apache.org>.
Github user echobravopapa commented on the issue:

    https://github.com/apache/geode-native/pull/83
  
    @dgkimura merged, please close


---
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 infrastructure@apache.org or file a JIRA ticket
with INFRA.
---