You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/05/16 20:52:18 UTC

[GitHub] [arrow] lidavidm opened a new pull request, #13169: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers

lidavidm opened a new pull request, #13169:
URL: https://github.com/apache/arrow/pull/13169

   Also, don't link every Arrow library to UCX when enabled.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #13169: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13169:
URL: https://github.com/apache/arrow/pull/13169#issuecomment-1128123877

   https://issues.apache.org/jira/browse/ARROW-16588


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] cyb70289 commented on a diff in pull request #13169: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers

Posted by GitBox <gi...@apache.org>.
cyb70289 commented on code in PR #13169:
URL: https://github.com/apache/arrow/pull/13169#discussion_r874380861


##########
cpp/CMakeLists.txt:
##########
@@ -827,11 +827,6 @@ if(ARROW_USE_XSIMD)
   list(APPEND ARROW_STATIC_LINK_LIBS xsimd)
 endif()
 
-if(ARROW_WITH_UCX)
-  list(APPEND ARROW_LINK_LIBS ucx::ucx)
-  list(APPEND ARROW_STATIC_LINK_LIBS ucx::ucx)
-endif()
-

Review Comment:
   It works. Thanks @kou!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] ursabot commented on pull request #13169: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #13169:
URL: https://github.com/apache/arrow/pull/13169#issuecomment-1129489058

   Benchmark runs are scheduled for baseline = 5f7a5a4574526da0493fd9dfa9e0a9c573bfcf3b and contender = a71e5f0d99992323639499dbeb8e5c1c6dd0fb0e. a71e5f0d99992323639499dbeb8e5c1c6dd0fb0e is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/7ab2f26da5fe43629c6c6e8a9814ca90...c071e03c538049b0bbaa36e8a18592f8/)
   [Finished :arrow_down:0.74% :arrow_up:0.04%] [test-mac-arm](https://conbench.ursa.dev/compare/runs/19355bd5d5b54a318d33b171268b38d6...bfbc7147f1e2454f94e78f7243490f7c/)
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/d9bb887d0d7a4946b628308812f7f6be...8733c6ba53f74ad8a4fe254ce582bee4/)
   [Finished :arrow_down:0.32% :arrow_up:0.04%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/768179515a2e4a78b060a5c6a158b4c5...bc9c54d891a141d9a49f5f01abc2dbce/)
   Buildkite builds:
   [Finished] [`a71e5f0d` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/778)
   [Finished] [`a71e5f0d` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/775)
   [Finished] [`a71e5f0d` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/765)
   [Finished] [`a71e5f0d` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/781)
   [Finished] [`5f7a5a45` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/777)
   [Finished] [`5f7a5a45` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/774)
   [Finished] [`5f7a5a45` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/764)
   [Finished] [`5f7a5a45` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/780)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] cyb70289 commented on a diff in pull request #13169: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers

Posted by GitBox <gi...@apache.org>.
cyb70289 commented on code in PR #13169:
URL: https://github.com/apache/arrow/pull/13169#discussion_r874297012


##########
cpp/CMakeLists.txt:
##########
@@ -827,11 +827,6 @@ if(ARROW_USE_XSIMD)
   list(APPEND ARROW_STATIC_LINK_LIBS xsimd)
 endif()
 
-if(ARROW_WITH_UCX)
-  list(APPEND ARROW_LINK_LIBS ucx::ucx)
-  list(APPEND ARROW_STATIC_LINK_LIBS ucx::ucx)
-endif()
-

Review Comment:
   Removing it causes bundled ucx build error.
   I'm not pretty sure of what this two lines does. Is it possible to include the headers somewhere else? @kou 
   
   ```
   In file included from ../src/arrow/flight/transport/ucx/flight_transport_ucx_test.cc:33:
   ../src/arrow/flight/transport/ucx/ucx_internal.h:27:10: fatal error: 'ucp/api/ucp.h' file not found
   #include <ucp/api/ucp.h>
            ^~~~~~~~~~~~~~~
   1 error generated.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou commented on a diff in pull request #13169: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers

Posted by GitBox <gi...@apache.org>.
kou commented on code in PR #13169:
URL: https://github.com/apache/arrow/pull/13169#discussion_r874343098


##########
cpp/CMakeLists.txt:
##########
@@ -827,11 +827,6 @@ if(ARROW_USE_XSIMD)
   list(APPEND ARROW_STATIC_LINK_LIBS xsimd)
 endif()
 
-if(ARROW_WITH_UCX)
-  list(APPEND ARROW_LINK_LIBS ucx::ucx)
-  list(APPEND ARROW_STATIC_LINK_LIBS ucx::ucx)
-endif()
-

Review Comment:
   Could you try the following patch?
   
   ```diff
   diff --git a/cpp/src/arrow/flight/transport/ucx/CMakeLists.txt b/cpp/src/arrow/flight/transport/ucx/CMakeLists.txt
   index 71392ec0af..d682ead633 100644
   --- a/cpp/src/arrow/flight/transport/ucx/CMakeLists.txt
   +++ b/cpp/src/arrow/flight/transport/ucx/CMakeLists.txt
   @@ -53,6 +53,7 @@ if(ARROW_BUILD_TESTS)
            arrow_flight_static
            arrow_flight_testing_static
            arrow_flight_transport_ucx_static
   +        ucx::ucx
            ${ARROW_TEST_LINK_LIBS})
      else()
        set(ARROW_FLIGHT_UCX_TEST_LINK_LIBS
   @@ -60,6 +61,7 @@ if(ARROW_BUILD_TESTS)
            arrow_flight_shared
            arrow_flight_testing_shared
            arrow_flight_transport_ucx_shared
   +        ucx::ucx
            ${ARROW_TEST_LINK_LIBS})
      endif()
      add_arrow_test(flight_transport_ucx_test
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] lidavidm commented on a diff in pull request #13169: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers

Posted by GitBox <gi...@apache.org>.
lidavidm commented on code in PR #13169:
URL: https://github.com/apache/arrow/pull/13169#discussion_r874818992


##########
cpp/CMakeLists.txt:
##########
@@ -827,11 +827,6 @@ if(ARROW_USE_XSIMD)
   list(APPEND ARROW_STATIC_LINK_LIBS xsimd)
 endif()
 
-if(ARROW_WITH_UCX)
-  list(APPEND ARROW_LINK_LIBS ucx::ucx)
-  list(APPEND ARROW_STATIC_LINK_LIBS ucx::ucx)
-endif()
-

Review Comment:
   Ah, I see. Thanks for the fix!



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #13169: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13169:
URL: https://github.com/apache/arrow/pull/13169#issuecomment-1128123891

   :warning: Ticket **has not been started in JIRA**, please click 'Start Progress'.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] cyb70289 commented on a diff in pull request #13169: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers

Posted by GitBox <gi...@apache.org>.
cyb70289 commented on code in PR #13169:
URL: https://github.com/apache/arrow/pull/13169#discussion_r874297012


##########
cpp/CMakeLists.txt:
##########
@@ -827,11 +827,6 @@ if(ARROW_USE_XSIMD)
   list(APPEND ARROW_STATIC_LINK_LIBS xsimd)
 endif()
 
-if(ARROW_WITH_UCX)
-  list(APPEND ARROW_LINK_LIBS ucx::ucx)
-  list(APPEND ARROW_STATIC_LINK_LIBS ucx::ucx)
-endif()
-

Review Comment:
   
   
   ```
   In file included from ../src/arrow/flight/transport/ucx/flight_transport_ucx_test.cc:33:
   ../src/arrow/flight/transport/ucx/ucx_internal.h:27:10: fatal error: 'ucp/api/ucp.h' file not found
   #include <ucp/api/ucp.h>
            ^~~~~~~~~~~~~~~
   1 error generated.
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kou closed pull request #13169: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers

Posted by GitBox <gi...@apache.org>.
kou closed pull request #13169: ARROW-16588: [C++][FlightRPC] Don't subclass GTest in test helpers
URL: https://github.com/apache/arrow/pull/13169


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org