You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "thisisnic (via GitHub)" <gi...@apache.org> on 2023/08/04 10:13:42 UTC
[GitHub] [arrow] thisisnic opened a new pull request, #37024: GH-36726: [R] calling read_parquet on S3 connections results in error message being ignored
thisisnic opened a new pull request, #37024:
URL: https://github.com/apache/arrow/pull/37024
### Rationale for this change
Valid error messages were being ignored and incorrect error messages were being displayed
### What changes are included in this PR?
I deleted a `tryCatch()` which was swallowing the error message. I don't know why it was there, and removing it didn't break any tests, so whatever behaviour it aims to supported is untested, and it may have been unnecessary.
### Are these changes tested?
No
### Are there any user-facing changes?
No
--
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] paleolimbot commented on a diff in pull request #37024: GH-36726: [R] calling read_parquet on S3 connections results in error message being ignored
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #37024:
URL: https://github.com/apache/arrow/pull/37024#discussion_r1289066619
##########
r/R/io.R:
##########
@@ -250,15 +250,8 @@ make_readable_file <- function(file, mmap = TRUE, random_access = TRUE) {
}
if (is_url(file)) {
- file <- tryCatch(
- {
- fs_and_path <- FileSystem$from_uri(file)
- fs_and_path$fs$OpenInputFile(fs_and_path$path)
- },
- error = function(e) {
- MakeRConnectionInputStream(url(file, open = "rb"))
- }
- )
+ fs_and_path <- FileSystem$from_uri(file)
+ file <- fs_and_path$fs$OpenInputFile(fs_and_path$path)
Review Comment:
I believe this will cause reading from an http(s) URL to fail where previously it worked for IPC and CSV:
``` r
arrow::read_ipc_stream("https://github.com/apache/arrow-testing/raw/master/data/arrow-ipc-stream/integration/1.0.0-littleendian/generated_primitive.stream") |> nrow()
#> [1] 37
```
Obviously that was a bad design on my part when I added this! Probably a better approach would be to add `if(is_http_url(file)) { file <- MakeRConnectionInputStream(url(file, open = "rb")) }` above this.
--
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] paleolimbot commented on pull request #37024: GH-36726: [R] calling read_parquet on S3 connections results in error message being ignored
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on PR #37024:
URL: https://github.com/apache/arrow/pull/37024#issuecomment-1679004244
CI failure seems to be due to an HTTP timeout
--
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] thisisnic merged pull request #37024: GH-36726: [R] calling read_parquet on S3 connections results in error message being ignored
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic merged PR #37024:
URL: https://github.com/apache/arrow/pull/37024
--
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] thisisnic commented on a diff in pull request #37024: GH-36726: [R] calling read_parquet on S3 connections results in error message being ignored
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on code in PR #37024:
URL: https://github.com/apache/arrow/pull/37024#discussion_r1289906449
##########
r/R/io.R:
##########
@@ -250,15 +250,8 @@ make_readable_file <- function(file, mmap = TRUE, random_access = TRUE) {
}
if (is_url(file)) {
- file <- tryCatch(
- {
- fs_and_path <- FileSystem$from_uri(file)
- fs_and_path$fs$OpenInputFile(fs_and_path$path)
- },
- error = function(e) {
- MakeRConnectionInputStream(url(file, open = "rb"))
- }
- )
+ fs_and_path <- FileSystem$from_uri(file)
+ file <- fs_and_path$fs$OpenInputFile(fs_and_path$path)
Review Comment:
I don't mind updating the PR, as your explanation of what is needed makes it pretty clear to me - thanks! Will come back to this next week as I have a few other things higher on my to-do list for this week.
--
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] thisisnic commented on pull request #37024: GH-36726: [R] calling read_parquet on S3 connections results in error message being ignored
Posted by "thisisnic (via GitHub)" <gi...@apache.org>.
thisisnic commented on PR #37024:
URL: https://github.com/apache/arrow/pull/37024#issuecomment-1679020246
I've re-run it as it's the linter job and I don't want to break everyone else's builds by accident if I've missed something
--
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] conbench-apache-arrow[bot] commented on pull request #37024: GH-36726: [R] calling read_parquet on S3 connections results in error message being ignored
Posted by "conbench-apache-arrow[bot] (via GitHub)" <gi...@apache.org>.
conbench-apache-arrow[bot] commented on PR #37024:
URL: https://github.com/apache/arrow/pull/37024#issuecomment-1680438668
After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit dcf913a368b1075111a690ab2c3e7dcb85f758b2.
There were no benchmark performance regressions. 🎉
The [full Conbench report](https://github.com/apache/arrow/runs/15939913461) has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.
--
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 #37024: GH-36726: [R] calling read_parquet on S3 connections results in error message being ignored
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #37024:
URL: https://github.com/apache/arrow/pull/37024#issuecomment-1665370728
:warning: GitHub issue #36726 **has been automatically assigned in GitHub** to PR creator.
--
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