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 2021/06/19 18:11:36 UTC
[GitHub] [arrow] jonkeane opened a new pull request #10560: ARROW-13127: [R] Valgrind nightly errors
jonkeane opened a new pull request #10560:
URL: https://github.com/apache/arrow/pull/10560
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#discussion_r655401085
##########
File path: r/tests/testthat/test-RecordBatch.R
##########
@@ -571,7 +571,7 @@ test_that("RecordBatchReader to C-interface", {
# export the RecordBatchReader via the C-interface
stream_ptr <- allocate_arrow_array_stream()
- on.exit(delete_arrow_array_stream(stream_ptr))
+ on.exit(delete_arrow_array_stream(stream_ptr), add = TRUE)
Review comment:
yes, that's true — I put it here as a reminder that if we move it around / add another one we need to make sure to have it, though it is not strictly necessary.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#issuecomment-864444507
https://issues.apache.org/jira/browse/ARROW-13127
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
jonkeane commented on pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#issuecomment-864469026
@github-actions crossbow submit test-r-linux-valgrind
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on a change in pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#discussion_r655541933
##########
File path: r/tests/testthat/test-RecordBatch.R
##########
@@ -571,7 +571,7 @@ test_that("RecordBatchReader to C-interface", {
# export the RecordBatchReader via the C-interface
stream_ptr <- allocate_arrow_array_stream()
- on.exit(delete_arrow_array_stream(stream_ptr))
+ on.exit(delete_arrow_array_stream(stream_ptr), add = TRUE)
Review comment:
FWIW the `add = TRUE` argument does not exist in all versions of R that we support today (I can't remember whether it was added in 3.4 or 3.5).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#discussion_r655399329
##########
File path: r/tests/testthat/test-RecordBatch.R
##########
@@ -571,7 +571,7 @@ test_that("RecordBatchReader to C-interface", {
# export the RecordBatchReader via the C-interface
stream_ptr <- allocate_arrow_array_stream()
- on.exit(delete_arrow_array_stream(stream_ptr))
+ on.exit(delete_arrow_array_stream(stream_ptr), add = TRUE)
Review comment:
This is probably not worth changing, but I think `add = TRUE` is not required here, since this is the first `on.exit()` in the function, correct?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#discussion_r655401085
##########
File path: r/tests/testthat/test-RecordBatch.R
##########
@@ -571,7 +571,7 @@ test_that("RecordBatchReader to C-interface", {
# export the RecordBatchReader via the C-interface
stream_ptr <- allocate_arrow_array_stream()
- on.exit(delete_arrow_array_stream(stream_ptr))
+ on.exit(delete_arrow_array_stream(stream_ptr), add = TRUE)
Review comment:
yes, that's true — I put it here as a reminder that if we move it around / add another one we need to make sure to have it, though it is not strictly necessary.
##########
File path: r/tests/testthat/test-RecordBatch.R
##########
@@ -571,7 +571,7 @@ test_that("RecordBatchReader to C-interface", {
# export the RecordBatchReader via the C-interface
stream_ptr <- allocate_arrow_array_stream()
- on.exit(delete_arrow_array_stream(stream_ptr))
+ on.exit(delete_arrow_array_stream(stream_ptr), add = TRUE)
Review comment:
Aaah good catch, I've moved all of these to calling after we don't need the pointer anymore (with a comment). That also calls out _slightly more clearly_ that pointer cleanup is *not* part of these functions which is not bad
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane closed pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
jonkeane closed pull request #10560:
URL: https://github.com/apache/arrow/pull/10560
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] nealrichardson commented on a change in pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
nealrichardson commented on a change in pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#discussion_r655541933
##########
File path: r/tests/testthat/test-RecordBatch.R
##########
@@ -571,7 +571,7 @@ test_that("RecordBatchReader to C-interface", {
# export the RecordBatchReader via the C-interface
stream_ptr <- allocate_arrow_array_stream()
- on.exit(delete_arrow_array_stream(stream_ptr))
+ on.exit(delete_arrow_array_stream(stream_ptr), add = TRUE)
Review comment:
FWIW the `add = TRUE` argument does not exist in all versions of R that we support today (I can't remember whether it was added in 3.4 or 3.5).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#issuecomment-865195293
Revision: e1b845d7e69662fb584aedeb9e619261096f8b5c
Submitted crossbow builds: [ursacomputing/crossbow @ actions-505](https://github.com/ursacomputing/crossbow/branches/all?query=actions-505)
|Task|Status|
|----|------|
|test-r-linux-valgrind|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-505-azure-test-r-linux-valgrind)|
|test-r-versions|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-505-github-test-r-versions)|
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane closed pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
jonkeane closed pull request #10560:
URL: https://github.com/apache/arrow/pull/10560
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] ianmcook commented on a change in pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
ianmcook commented on a change in pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#discussion_r655399329
##########
File path: r/tests/testthat/test-RecordBatch.R
##########
@@ -571,7 +571,7 @@ test_that("RecordBatchReader to C-interface", {
# export the RecordBatchReader via the C-interface
stream_ptr <- allocate_arrow_array_stream()
- on.exit(delete_arrow_array_stream(stream_ptr))
+ on.exit(delete_arrow_array_stream(stream_ptr), add = TRUE)
Review comment:
This is probably not worth changing, but I think `add = TRUE` is not required here, since this is the first `on.exit()` in the function, correct?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#issuecomment-864444588
Revision: a057f126683485806e0e090e1d7f0b12800125d2
Submitted crossbow builds: [ursacomputing/crossbow @ actions-503](https://github.com/ursacomputing/crossbow/branches/all?query=actions-503)
|Task|Status|
|----|------|
|test-r-linux-valgrind|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-503-azure-test-r-linux-valgrind)|
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
jonkeane commented on pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#issuecomment-865194767
@github-actions crossbow submit test-r-versions test-r-linux-valgrind
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
jonkeane commented on pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#issuecomment-865194767
@github-actions crossbow submit test-r-versions test-r-linux-valgrind
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on a change in pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
jonkeane commented on a change in pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#discussion_r655556102
##########
File path: r/tests/testthat/test-RecordBatch.R
##########
@@ -571,7 +571,7 @@ test_that("RecordBatchReader to C-interface", {
# export the RecordBatchReader via the C-interface
stream_ptr <- allocate_arrow_array_stream()
- on.exit(delete_arrow_array_stream(stream_ptr))
+ on.exit(delete_arrow_array_stream(stream_ptr), add = TRUE)
Review comment:
Aaah good catch, I've moved all of these to calling after we don't need the pointer anymore (with a comment). That also calls out _slightly more clearly_ that pointer cleanup is *not* part of these functions which is not bad
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#issuecomment-865195293
Revision: e1b845d7e69662fb584aedeb9e619261096f8b5c
Submitted crossbow builds: [ursacomputing/crossbow @ actions-505](https://github.com/ursacomputing/crossbow/branches/all?query=actions-505)
|Task|Status|
|----|------|
|test-r-linux-valgrind|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-505-azure-test-r-linux-valgrind)|
|test-r-versions|[](https://github.com/ursacomputing/crossbow/actions?query=branch:actions-505-github-test-r-versions)|
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jonkeane commented on pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
jonkeane commented on pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#issuecomment-864444513
@github-actions crossbow submit test-r-linux-valgrind
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #10560: ARROW-13127: [R] Valgrind nightly errors
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #10560:
URL: https://github.com/apache/arrow/pull/10560#issuecomment-864469114
Revision: 31fc14eeb7ca55e24fdf6fc01f506e5988a30c7d
Submitted crossbow builds: [ursacomputing/crossbow @ actions-504](https://github.com/ursacomputing/crossbow/branches/all?query=actions-504)
|Task|Status|
|----|------|
|test-r-linux-valgrind|[](https://dev.azure.com/ursacomputing/crossbow/_build/latest?definitionId=1&branchName=actions-504-azure-test-r-linux-valgrind)|
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org