Conversation
|
Would you mind pointing me to the BigQuery docs for labels? In the interest of using the simplest possible data structure, is there a reason to use a list instead of a named character vector? |
…s and tests for check_labels function
Here are the docs: https://docs.cloud.google.com/bigquery/docs And you are correct, the list was unnecessary. I have changed that to a named chr vector. |
|
Adding my support for this feature – it is important for cost allocation in production applications that use R to communicate with BigQuery. |
hadley
left a comment
There was a problem hiding this comment.
Thanks for working on this! A few comments on the overall approach and implementation below.
I assume you have tested this interactively?
| check_bool(print_header) | ||
| check_string(billing) | ||
|
|
||
| labels <- check_labels(getOption("bigrquery.labels")) |
There was a problem hiding this comment.
As a general principle, I don't think it's good idea to allow global options to change what a computation does, only how it looks. I think it's probably ok in this case because I think you can frame this as a sort of logging operation, but it should appear in function arguments too.
I would also wonder if it would instead make sense to make it a field in BigQueryConnection. Would that also solve the problem for you or are you usually calling these low-level functions directly?
There was a problem hiding this comment.
I understand.
I do use the low-level functions directly currently, but it is also a good point with a field in BigQueryConnection.
Would it make sense to both:
- Add labels field to BigQueryConnection
- Add labels as arguments to each bq_perform_... function? - To set the labels explicitly
There was a problem hiding this comment.
I'm also happy to do that myself, if you don't have the time.
Ofc, thank you the comments! We use this every day, but I have now tested it more thoroughly. I had to change the character vector back to list - i hadn't tested that change well enough. |
…eck_labels and corrected tests
Added labels to job creation calls for FinOps purposes
Labels can be set globally via:
options(bigrquery.labels = list(env = "prod", team = "analytics"))
Labels are then automatically attached to all BigQuery job requests (query, load, extract, copy).
Fixes #652