Skip to content

Conversation

@bkietz
Copy link
Member

@bkietz bkietz commented Mar 25, 2020

No description provided.

@github-actions
Copy link

@bkietz bkietz requested a review from fsaintjacques March 26, 2020 02:44
@bkietz bkietz force-pushed the 8164-Let-datasets-be-viewable- branch 2 times, most recently from 0f58a1f to 507bdb6 Compare March 30, 2020 15:16
@fsaintjacques
Copy link
Contributor

fsaintjacques commented Mar 31, 2020

Can you rebase with master, there's a lot of failing test noise. My approval is pending at least a checkup of the failures :)

@bkietz bkietz force-pushed the 8164-Let-datasets-be-viewable- branch from 507bdb6 to 0c0fdad Compare March 31, 2020 16:33
@bkietz
Copy link
Member Author

bkietz commented Mar 31, 2020

CI failures are s3fs, grpc build failure x2 x3, and RTools flake

@nealrichardson
Copy link
Member

It's not clear that the R Windows failure is a flake, it reads similar to https://issues.apache.org/jira/browse/ARROW-8217

@bkietz bkietz force-pushed the 8164-Let-datasets-be-viewable- branch from 0c0fdad to 0d117ff Compare April 1, 2020 18:36
@jorisvandenbossche
Copy link
Member

There is still a R failure (don't know whether this is related), and windows is failing in cmake (related to googletest)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I read this diff correctly, the only place that the R package tests should touch new code (i.e. without calling ReplaceSchema, which has a binding added but no test) is here. Is there something suspicious about CheckProjectable (on 32-bit mingw)?

@nealrichardson
Copy link
Member

R is passing on master, so we need to figure out why it's failing here.

@nealrichardson nealrichardson force-pushed the 8164-Let-datasets-be-viewable- branch from 0d117ff to 46b6f4c Compare April 3, 2020 15:36
@nealrichardson
Copy link
Member

nealrichardson commented Apr 3, 2020

Reproduced the crash locally. https://github.com/apache/arrow/blob/master/r/tests/testthat/test-json.R#L30 is where it exits. Can't get a backtrace with gdb, even with a debug build.

@nealrichardson
Copy link
Member

@bkietz Here's an even shorter repro, reduced from the test that blows up:

library(arrow)
tf <- tempfile()
writeLines('{ "hello": 3.5 }\n', tf, useBytes = TRUE)
read_json_arrow(tf, as_data_frame = FALSE)

@nealrichardson
Copy link
Member

I did some more interactive exploration of this in R, and found that the crash happens when calling arrow::json::TableReader's Read() method.

@bkietz bkietz force-pushed the 8164-Let-datasets-be-viewable- branch 3 times, most recently from 2fb5e27 to 173105b Compare April 8, 2020 18:21
@nealrichardson nealrichardson force-pushed the 8164-Let-datasets-be-viewable- branch from 91ef2b2 to d9ab1bf Compare April 8, 2020 22:28
@nealrichardson
Copy link
Member

🎉 @bkietz the Rtools build is passing now! But now 6 other things are failing. Should check if they're flakes or if we broke something else in this flailing.

@nealrichardson
Copy link
Member

merging 🤞

@bkietz bkietz deleted the 8164-Let-datasets-be-viewable- branch February 25, 2021 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants