-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add fromServiceAccountJsonInputStream #7
base: master
Are you sure you want to change the base?
add fromServiceAccountJsonInputStream #7
Conversation
…rvice account json
@seratch any thoughts? |
@@ -155,7 +155,17 @@ object BigQuery { | |||
scopes: Seq[String] = Seq(BigqueryScopes.BIGQUERY) | |||
): BigQuery = { | |||
|
|||
val credential = using(new FileInputStream(serviceAccountJsonFilePath)) { in => | |||
BigQuery.fromServiceAccountJsonInputStream(new FileInputStream(serviceAccountJsonFilePath), transport, jsonFactory, scopes) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the FileInputStream
resource must be closed.
scopes: Seq[String] = Seq(BigqueryScopes.BIGQUERY) | ||
): BigQuery = { | ||
|
||
val credential = using(serviceAccountJsonInputStream) { in => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, the passed resource is closed here. I don't think it's a good approach. Closing a resource near its creation is easy to maintain.
…Stream is created
hey @seratch I was on holiday and missed your comments. I switched things back a bit so the existing |
similar to the
BigQuery.fromServiceAccountJson
(that expects a file path), this new method allows you to pass in anInputStream
. We have a use-case where all configuration is happening via ENV vars, and I'd prefer to not have to write the config to the local (container) filesystem.I also added a test for both of the service account json methods I have added. Same as the existing test, it requires you change
yourOwnProjectId
and to have a file existing at$HOME/.bigquery/service_account.json
: