feat(bigquery): Add cloud-client samples for access policies by hivanalejandro · Pull Request #3975 · GoogleCloudPlatform/nodejs-docs-samples

hivanalejandro · 2025-02-13T20:01:14Z

Description

Add sample for viewing access policy to dataset

This PR has been divided into the following 3:

#4023
#4024
#4025

Step of Internal: b/394478489

Checklist

I have followed guidelines from CONTRIBUTING.MD and Samples Style Guide
Tests pass: npm test (see Testing)
Lint pass: npm run lint (see Style)
Please merge this PR for me once it is approved

- Configure main app.js entry point - Configure viewDatasetAccessPolicy.js file

- Add viewTableOrViewAccessPolicy to the app.js entry point - Configure viewTableOrViewAccessPolicy.js file

snippet-bot · 2025-02-13T20:01:22Z

Here is the summary of changes.

You are about to add 6 region tags.

bigquery/cloud-client/grantAccessToDataset.js:26, tag bigquery_grant_access_to_dataset
bigquery/cloud-client/grantAccessToTableOrView.js:34, tag bigquery_grant_access_to_table_or_view
bigquery/cloud-client/revokeDatasetAccess.js:27, tag bigquery_revoke_dataset_access
bigquery/cloud-client/revokeTableOrViewAccess.js:33, tag bigquery_revoke_access_to_table_or_view
bigquery/cloud-client/viewDatasetAccessPolicy.js:23, tag bigquery_view_dataset_access_policy
bigquery/cloud-client/viewTableOrViewAccessPolicy.js:26, tag bigquery_view_table_or_view_access_policy

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

Refresh this comment

…ataset-access-policy

…ix linting errors

…eOrViewAccess.test.js

…ataset-access-policy

eapl-gemugami

I left a few comments. Please let me know if you require further details.

…ataset-access-policy

eapl-gemugami

I'm leaving a few comments, let me know if you need further details!

…ataset-access-policy

eapl-gemugami

A few comments for the latest commit.

…ataset-access-policy

eapl-gemugami

I'm sharing a few comments on Sample formatting and scope of used variables inside the region/block.

eapl-gemugami · 2025-03-06T15:58:38Z

+    "version": "0.0.1",
+    "private": true,
+    "license": "Apache Version 2.0",
+    "author": "Google Inc.",


From what I've seen, the author should be 'Google LLC', as in:

nodejs-docs-samples/.mocharc.js

Line 1 in e542724

// Copyright 2020 Google LLC

I was checking the package.json files of other samples and all of them have Google Inc. in the author part.

These reference package.json files using Google Inc. might be outdated, see renderer/package.json#L6 from 2020 or retail/package.json#L4) from 2021

Also check b/169619920

Anyway, you could ask your NodeJS lead for advice.

…documents

…ataset-access-policy

eapl-gemugami

Comments for minor details found in last commit.

eapl-gemugami · 2025-03-06T22:51:49Z

+    "version": "0.0.1",
+    "private": true,
+    "license": "Apache Version 2.0",
+    "author": "Google Inc.",


These reference package.json files using Google Inc. might be outdated, see renderer/package.json#L6 from 2020 or retail/package.json#L4) from 2021

Also check b/169619920

eapl-gemugami · 2025-03-06T22:52:23Z

+    "version": "0.0.1",
+    "private": true,
+    "license": "Apache Version 2.0",
+    "author": "Google Inc.",


Anyway, you could ask your NodeJS lead for advice.

eapl-gemugami · 2025-03-06T23:02:46Z

+  }
+
+  if (principalToRemove) {
+    // The `bindings` list is immutable. Create a copy for modifications.


Question:
I added that comment on the Python Sample to warn the developer that they can't modify the list but have to create a new one, as working on the current list was the 'obvious' thing to do.

That was required on Python, after some research on the source code and struggling to make the sample work, is it the same case for Node?
My only concern would be to create a new list if it's not necessary on Node cloud-client.

Unlike Python, creating a new list in Node is not strictly necessary for this operation to work. JS doesn't have the same restrictions as Python when modifying list during iteration.

However, creating a copy with const bindings = [...policy.bindings]; is still recommended because it prevents potential issues if the BQ Node client has any internal expectations about immutability of the policy object.

Just for the record: Besides having problems modifying a list during an iteration, the list on Python is immutable due to the way the bindings were designed:
iam.py#L141-L148

From what I can see here types.d.ts#L3683 and there types.d.ts#L1752-L1756, Node client is more relaxed on how to work the list, but I can't confirm if it's immutable.

Anyway, if you are not sure about the binding list being immutable, to avoid misleading the developer you could just leave:
// Create a copy of the binding list for modifications.

…ataset-access-policy

eapl-gemugami

LGTM

hivanalejandro added 4 commits February 13, 2025 18:51

feat(bigquery): initial project setup

5925b0f

feat(bigquery): basic structure

e24c2e8

- Configure main app.js entry point - Configure viewDatasetAccessPolicy.js file

feat(bigquery): Add table and view access policy viewer

84ec0f6

- Add viewTableOrViewAccessPolicy to the app.js entry point - Configure viewTableOrViewAccessPolicy.js file

feat(bigquery): Add viewDatasetAccessPolicy tests

21f786e

hivanalejandro requested review from a team as code owners February 13, 2025 20:01

product-auto-label Bot added api: bigquery Issues related to the BigQuery API. samples Issues that are directly related to samples. labels Feb 13, 2025

hivanalejandro marked this pull request as draft February 13, 2025 20:01

hivanalejandro requested a review from eapl-gemugami February 13, 2025 20:02

hivanalejandro added 14 commits February 14, 2025 14:56

feat(bigquery): Add viewTableOrViewAccessPolicy tests

5d408c7

fix(bigquery): Fix linting errors

62e343a

Merge branch 'main' into hivanalejandro/bigquery/create-sample/view-d…

c87d659

…ataset-access-policy

feat(bigquery): Add revokeTableOrViewAccess feawture and tests

58ee72a

feat(bigquery): Update app.js file to add revokeTableOrViewAccess & f…

62ebc2c

…ix linting errors

fix(bigquery):Fix headers for revokeTableOrViewAccess.js & revokeTabl…

9d0ce88

…eOrViewAccess.test.js

feat(bigquery): Add bigquery_revoke_access_to_table_or_view tag

627536a

fix(bigquery): Update if/else to if/else if

70cfb83

feat(bigquery): Add grantAccessToDataset sample and tests

ff35988

Merge branch 'main' into hivanalejandro/bigquery/create-sample/view-d…

b8ad0f4

…ataset-access-policy

fix(bigquery): Update lint error

7b38e5a

feat(bigquery): Add grantAccessToTableOrView sample

5cfa8dc

feat(bigquery): Add grantAccessToTableOrView test

ccf30d6

feat(bigquery): Update app.js file with new samples

eb32aa8

hivanalejandro changed the title ~~feat(bigquery): WIP create sample 'viewTableOrViewAccessPolicy' & 'viewDatasetAccessPolicy'~~ feat(bigquery): WIP feat(bigquery): add samples for access policies Feb 20, 2025

hivanalejandro changed the title ~~feat(bigquery): WIP feat(bigquery): add samples for access policies~~ feat(bigquery): WIP add samples for access policies Feb 20, 2025

hivanalejandro added 3 commits February 20, 2025 18:28

feat(bigquery): Add revokeDatasetAccess sample

0a5f5dd

feat(bigquery): Add revokeDatasetAccess tests

6c40940

feat(bigquery): Update app.js file with new sample

b600729

hivanalejandro requested review from OremGLG and iennae February 20, 2025 20:24

eapl-gemugami reviewed Feb 20, 2025

View reviewed changes

hivanalejandro added 3 commits February 27, 2025 18:41

fix(bigquery): Update samples and tests related to Dataset

b013b3c

Merge branch 'main' into hivanalejandro/bigquery/create-sample/view-d…

3f3d68a

…ataset-access-policy

fix(bigquery): Update format issues

12bc84b

hivanalejandro requested a review from eapl-gemugami February 27, 2025 19:16

eapl-gemugami reviewed Feb 27, 2025

View reviewed changes

hivanalejandro added 3 commits March 3, 2025 18:37

fix(bigquery): Update samples and tests related to Table or View

b9e4bb1

Merge branch 'main' into hivanalejandro/bigquery/create-sample/view-d…

e6e886f

…ataset-access-policy

fix(bigquery): Update samples and tests according to PR comments

ec3c990

hivanalejandro requested a review from eapl-gemugami March 4, 2025 13:48

Merge branch 'main' into hivanalejandro/bigquery/create-sample/view-d…

232b73b

…ataset-access-policy

eapl-gemugami reviewed Mar 4, 2025

View reviewed changes

hivanalejandro added 3 commits March 5, 2025 15:45

fix(bigquery): Update samples and tests format issues

7c45228

Merge branch 'main' into hivanalejandro/bigquery/create-sample/view-d…

f6a685d

…ataset-access-policy

fix(bigquery): Update samples and tests according to PR comments

dbb3a4d

hivanalejandro requested a review from eapl-gemugami March 5, 2025 16:56

hivanalejandro marked this pull request as ready for review March 5, 2025 16:56

fix(bigquery): Standardized punctuation and style in all documents

5c04884

eapl-gemugami reviewed Mar 6, 2025

View reviewed changes

hivanalejandro added 2 commits March 6, 2025 17:43

fix(bigquery): Add punctuation to test and standardize format in all …

eed264a

…documents

Merge branch 'main' into hivanalejandro/bigquery/create-sample/view-d…

20609bf

…ataset-access-policy

eapl-gemugami reviewed Mar 6, 2025

View reviewed changes

hivanalejandro added 2 commits March 7, 2025 19:22

Merge branch 'main' into hivanalejandro/bigquery/create-sample/view-d…

f721a80

…ataset-access-policy

fix(bigquery): Update samples and tests according to PR comments

158f0b0

hivanalejandro requested a review from eapl-gemugami March 7, 2025 19:30

hivanalejandro changed the title ~~feat(bigquery): WIP add samples for access policies~~ feat(bigquery): Add cloud-client samples for access policies Mar 7, 2025

eapl-gemugami approved these changes Mar 7, 2025

View reviewed changes

hivanalejandro closed this Mar 8, 2025

Conversation

hivanalejandro commented Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

This PR has been divided into the following 3:

Checklist

Uh oh!

snippet-bot Bot commented Feb 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

eapl-gemugami left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eapl-gemugami left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eapl-gemugami left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eapl-gemugami left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eapl-gemugami Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

hivanalejandro Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

eapl-gemugami Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eapl-gemugami Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

eapl-gemugami left a comment

Choose a reason for hiding this comment

Uh oh!

eapl-gemugami Mar 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eapl-gemugami Mar 6, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

hivanalejandro commented Feb 13, 2025 •

edited

Loading

snippet-bot Bot commented Feb 13, 2025 •

edited

Loading

eapl-gemugami Mar 6, 2025 •

edited

Loading

eapl-gemugami Mar 6, 2025 •

edited

Loading

eapl-gemugami Mar 6, 2025 •

edited

Loading