Skip to content

Craig Citro
craigcitro

Organizations

@google @GoogleCloudPlatform @googlegenomics @grpc @cxxr-devel @gflags @tensorflow
craigcitro commented on issue craigcitro/r-travis#172
@craigcitro

@jimhester didn't you just fix this for native travis builds? is that something we could easily backport here?

craigcitro commented on pull request rstats-db/bigrquery#87
@craigcitro

One small nit, and the standard question -- can we add a test? :grin:

craigcitro commented on pull request rstats-db/bigrquery#87
@craigcitro

it might be worth taking this as an argument to list_tabledata_iter, even if just for testing purposes.

craigcitro commented on pull request rstats-db/bigrquery#88
@craigcitro

LGTM -- is this easy enough to add a test for?

@craigcitro

rather than nest these try blocks, can we just have a loop over the set of uri templates?

@craigcitro

also pinging @ebrevdo who may have hit this more recently.

@craigcitro

@maadcodr I can't seem to repro this -- let's move to a new issue, and include some details about your setup (at least OS, docker version)?

craigcitro commented on issue rstats-db/bigrquery#86
@craigcitro

what code were you trying to run?

@craigcitro

I agree that getting the container started with GPU support still isn't the smoothest experience, especially when it comes to matching library vers…

@craigcitro
@craigcitro
craigcitro merged pull request rstats-db/bigrquery#81
@craigcitro
New format_dataset() and format_table()
12 commits with 193 additions and 53 deletions
craigcitro commented on pull request rstats-db/bigrquery#81
@craigcitro

LGTM, happy to resolve the other bits separately

craigcitro commented on issue rstats-db/bigrquery#85
@craigcitro

If bigrquery existed in the abstract, i agree using just project, dataset, and table is a clean choice. However, in our case, I think it's worth ma…

craigcitro commented on pull request rstats-db/bigrquery#46
@craigcitro

@ericbrownaustin Can you say more about what you're looking to do? There are disposition arguments on copy, but in general you can just create the …

craigcitro deleted branch copy at craigcitro/bigrquery
@craigcitro
@craigcitro
@craigcitro
  • @craigcitro 6766dce
    Add a test for more coverage.
@craigcitro
  • @craigcitro 06d118b
    Turn on coverage via covr and codecov.
@craigcitro
craigcitro merged pull request rstats-db/bigrquery#80
@craigcitro
Extract run_query_job()
1 commit with 13 additions and 3 deletions
craigcitro commented on pull request rstats-db/bigrquery#81
@craigcitro

LGTM to me modulo comments above. totally fine with moving these all out of utils.R.

craigcitro commented on pull request rstats-db/bigrquery#81
@craigcitro

it's strange that i apparently didn't add a dataset_id and use it if necessary. seems like it'd be worth adding?

craigcitro commented on pull request rstats-db/bigrquery#81
@craigcitro

should we tweak the args here, so that do.call(format_dataset, parse_dataset(...)) == ...

craigcitro commented on pull request rstats-db/bigrquery#81
@craigcitro

identifier.

craigcitro commented on pull request rstats-db/bigrquery#81
@craigcitro

small nit: i think this is easier to read when each assertion is on its own line, eg assert_that(is.string(dataset), is_null(project_id) || is.stri…

@craigcitro

Sending a CL out now.

@craigcitro
craigcitro merged pull request rstats-db/bigrquery#83
@craigcitro
Removed unused argument.
1 commit with 1 addition and 1 deletion
Something went wrong with that request. Please try again.