search

GoogleCloudPlatform / healthcare-federated-access-services

Apache License 2.0
25 stars 9 forks source link

Added support for specifying a separate project to run bq jobs #31

Closed monicavalluri closed 4 years ago

monicavalluri commented 4 years ago

Add support for specifying a different project for running BQ jobs. This prevents granting Security Admin in the project that the data resides in. This PR has changes from https://github.com/GoogleCloudPlatform/healthcare-federated-access-services/pull/9

monicavalluri commented 4 years ago

@chaopeng could you please help me with the build logs, I do not have permissions to view the logs.

chaopeng commented 4 years ago

I also don't have permission. I think this is not build logs.

cdvoisin commented 4 years ago

The build logs are cryptic here, with a number of errors of the form: failed: invalid character 'p' after top-level value

For example:

Step #3 - "go_test": === RUN TestConfigHandlers Step #3 - "go_test": E0514 15:18:24.867239 2046 dam.go:467] failed to sync hydra clients: rpc error: code = Internal desc = DecodeJSON() failed: json.Unmarshal(404 page not found Step #3 - "go_test": ) failed: invalid character 'p' after top-level value Step #3 - "go_test": === RUN TestConfigHandlers/#00 ...

Could it be that ") failed" is trying to tell us what failed, but it is getting ")" as the name of the item?

We get this same error 17 times across several tests: TestConfigHandlers, TestHandlers, TestMinConfig, etc. This would be expected if loading of configs such as the adapter_saw.json file were not parsable. However, my read of this config change didn't find any errors, let alone an invalid 'p' after the top level. The protos haven't changed, so it isn't a gen_protobuf.bash problem.

@chaopeng maybe we can mock up a PR with just the config change and see if we can reproduce the problem? Also, would be good to fix the ") failed" message to indicate what file it is processing and any other context about what operation it is trying to perform.

chaopeng commented 4 years ago

@mbarkley sorry, this PR has been merged. Please start another PR for https://github.com/DNAstack/healthcare-federated-access-services/commit/0d18bf6787832d5fb74f0fe50e94d31c9edf2e55 commit