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
Fix issue #60: keyFile in config (#60) #61
base: master
Are you sure you want to change the base?
Conversation
Fix logic if keyFile is null
return new StorageClient([ | ||
'projectId' => $config['project_id'], | ||
'keyFilePath' => $keyFile, | ||
'keyFile' => array_merge(["project_id" => $config['project_id']], $keyFile) |
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.
we can actually skip this whole is_array
check entirely and just remove the array_merge
. The key file should already contain project_id
, so I'm not sure why it needs to be merged in the first place?
@nicja thoughts on trying to get some tests in here? I'm wondering if @ralfiannor should perhaps at least add a test for the config merging, but that would require some setup on your side to get a pipeline running. |
Confirmed that this fix works for a GKE-powered environment with creds provided at the instance level. Had the same issue mentioned in #68. I'll fork off of this branch and a test that fails prior to this commit and succeeds with the commit if that's what it takes to get this merged. |
Fix logic if keyFile is null