-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feature(bigquery): add features for BigQuery access policies #5144
base: main
Are you sure you want to change the base?
feature(bigquery): add features for BigQuery access policies #5144
Conversation
…adiaz-features-bigquery
…ciadiaz-features-bigquery
…rciadiaz-features-bigquery
|
||
ctx := context.Background() | ||
|
||
// Creates bq client. |
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.
I'd suggest not using bq
, as it could be confusing for newcomers, perhaps "BigQuery client", or only "Client".
For Python I'm using:
# Instantiate a client.
bigquery_client = bigquery.Client()
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.
Thanks, I'll update the comments
} | ||
|
||
// Table schema. | ||
sampleSchema := bigquery.Schema{ |
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.
If the test doesn't require a specific table schema, you could use something simpler.
For Python I'm using
sample_schema = [
bigquery.SchemaField("id", "INTEGER", mode="REQUIRED"),
]
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.
I think that you meant to create a simplier schema, because the schema should be created anyways. I'll make it simplier
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.
Agreed, I've reworded for clarity, thanks!
|
||
// Sets view query. | ||
viewMetadata := &bigquery.TableMetadata{ | ||
ViewQuery: fmt.Sprintf("SELECT UPPER(full_name) FROM `%s.%s` ORDER BY age ASC", datasetName, tableName), |
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.
Following my previous comment, as the tests need to be as simple as possible (as per David's comment), I'd suggest going with something like
view.view_query = f"SELECT * FROM
{table}"
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.
Sure, following that
func TestGrantAccessTable(t *testing.T) { | ||
tc := testutil.SystemTest(t) | ||
|
||
datasetName := "my_new_dataset_go" |
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.
Check your best practices for Go's testing. I don't think 'my_new_dataset_go' is the right name in this test, as sounds more suited for samples. I'd advise of using a name related with the sample you are running.
In Python a Prefixer is being used to avoid possible conflicts among test runs:
PREFIX = prefixer.create_prefix()
DATASET_ID = f"{PREFIX}_view_access_policies"
TABLE_NAME = f"{PREFIX}_view_access_policies_table"
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.
Thanks for the suggestion. It makes sense if tests are running concurrently
Description
Add sample for viewing access policy to dataset
Step of Internal: b/394478489
https://cloud.google.com/bigquery/docs/control-access-to-resources-iam#view_the_access_policy_of_a_dataset
https://cloud.google.com/bigquery/docs/control-access-to-resources-iam#view_the_access_policy_of_a_table_or_view
https://cloud.google.com/bigquery/docs/control-access-to-resources-iam#grant_access_to_a_table_or_view
https://cloud.google.com/bigquery/docs/control-access-to-resources-iam#revoke_access_to_a_table_or_view
Checklist
go test -v ./..
(see Testing)gofmt
(see Formatting)go vet
(see Formatting)