Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add Dataset access support #1046

Merged
merged 9 commits into from
Aug 11, 2015
Merged

Add Dataset access support #1046

merged 9 commits into from
Aug 11, 2015

Conversation

tseaver
Copy link
Contributor

@tseaver tseaver commented Aug 7, 2015

Uses #1045 as a base.

  • Adds Dataset.access_grants property.
  • Marshals values to/from that property in Dataset API requests/responses.

@tseaver tseaver added the api: bigquery Issues related to the BigQuery API. label Aug 7, 2015
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Aug 7, 2015
@tseaver tseaver mentioned this pull request Aug 7, 2015
Merged
@dhermes
Copy link
Contributor

dhermes commented Aug 8, 2015

Is an AccessGrant like an ACL or something different?

dhermes
dhermes reviewed Aug 8, 2015
View reviewed changes
gcloud/bigquery/dataset.py
self.entity_type = entity_type
self.entity_id = entity_id

def __repr__(self): # pragma: NO COVER

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@dhermes
Copy link
Contributor

dhermes commented Aug 8, 2015

Is there a standalone ACL API for this like there is for storage?

dhermes
dhermes reviewed Aug 8, 2015
View reviewed changes
gcloud/bigquery/dataset.py
entity_type = 'specialGroup'
elif 'groupByEmail' in grant:
entity_type = 'groupByEmail'
else:

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver
Copy link
Contributor Author

tseaver commented Aug 8, 2015

Is an AccessGrant like an ACL or something different?

An AccessGrant instance is a little like a single entity in an ACL. Unlike some ACL systems. this system does not have a notion of a "deny permission" / "allow permission" entry: all the entries are grants of a role (enabling some kind of access) to a principal.

Is there a standalone ACL API for this like there is for storage?

Nope.

dhermes
dhermes reviewed Aug 10, 2015
View reviewed changes
gcloud/bigquery/dataset.py
for grant in access:
grant = grant.copy()
role = grant.pop('role')
entity_type, entity_id = list(grant.items())[0]

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

@tseaver
Copy link
Contributor Author

tseaver commented Aug 10, 2015

Rebased after merge of #1045.

@tseaver
Work around hypothetical case parsing 'dataset.access_grants':
09c1d14 
The back-end might return a grant with more than one entity_type for a given
role.

See:
#1046 (comment)
@tseaver
Copy link
Contributor Author

tseaver commented Aug 11, 2015

@dhermes Any remaining issues?

@dhermes
Copy link
Contributor

dhermes commented Aug 11, 2015

@tseaver We still have an unresolved question to @fhoffa. I'm not sure 09c1d14 does the trick.

Also note the failure on Travis

======================================================================
FAIL: test__parse_access_grants_w_multiple_entity_types (gcloud.bigquery.test_dataset.TestDataset)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/GoogleCloudPlatform/gcloud-python/gcloud/bigquery/test_dataset.py", line 284, in test__parse_access_grants_w_multiple_entity_types
    self._verifyAccessGrants(grants, RESOURCE)
  File "/home/travis/build/GoogleCloudPlatform/gcloud-python/gcloud/bigquery/test_dataset.py", line 88, in _verifyAccessGrants
    self.assertEqual(a_grant.entity_type, r_grant['entity_type'])
AssertionError: 'specialGroup' != 'userByEmail'
- specialGroup
+ userByEmail

Is it possible the test is non-deterministic / depends on dictionary order?

@tseaver
Defend against hash randomization non-determanism.
6720b35 
Also, de-lint long URL comments.
@tseaver
Copy link
Contributor Author

tseaver commented Aug 11, 2015

AFAICT, 09c1d14 should handle cases where the back-end returns the hypothetical multiple grant. We can back it out if @fhoffa says it can never happen.

Yes, the failure is likely due to hash randomization. 6720b35 should fix that.

@dhermes
Copy link
Contributor

dhermes commented Aug 11, 2015

Let's get this in so we can get a BQ release, but I really do not want to drop this question. Calling sorted does not seem to be the right move.

LGTM

@dhermes dhermes mentioned this pull request Aug 11, 2015
Closed
@dhermes
Copy link
Contributor

dhermes commented Aug 11, 2015

#1050 opened to continue this discussion

tseaver added a commit that referenced this pull request Aug 11, 2015
@tseaver
Merge pull request #1046 from tseaver/bigquery-dataset_acls
2d017de 
Add Dataset access support
@tseaver tseaver merged commit 2d017de into googleapis:master Aug 11, 2015
@tseaver tseaver deleted the bigquery-dataset_acls branch August 11, 2015 18:59
@dhermes dhermes mentioned this pull request Aug 12, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@tseaver @dhermes @googlebot