Skip to content
This repository was archived by the owner on Mar 6, 2026. It is now read-only.

fix: make AccessEntry equality consistent with from_api_repr #2218

Merged
Linchin merged 11 commits into from
Jun 26, 2025
Merged

fix: make AccessEntry equality consistent with from_api_repr #2218

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
969f09f
fix: make AccessEntry equality consistent for view entity type
drokeye Jun 18, 2025
0dc6b35
fix: make AccessEntry equality consistent for view entity type
drokeye Jun 18, 2025
7687693
fix: use json.dumps() for normalizaiton of entity_id
drokeye Jun 19, 2025
6ab2c3c
remove trailing whitespace and add test assertions
drokeye Jun 21, 2025
33c01b5
Merge branch 'main' into fix-accessentry-equality
Linchin Jun 23, 2025
6baa50b
revert back to the original code
drokeye Jun 24, 2025
949aa3c
Merge branch 'fix-accessentry-equality' of https://github.com/drokeye…
drokeye Jun 24, 2025
266989a
Merge branch 'main' into fix-accessentry-equality
Linchin Jun 24, 2025
adffcf5
fix linting in `dataset.py`
drokeye Jun 25, 2025
4230a9d
fix linting in `test_dataset.py`
drokeye Jun 25, 2025
12fae55
Merge branch 'main' into fix-accessentry-equality
Linchin Jun 26, 2025
File filter

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
remove trailing whitespace and add test assertions
  • Loading branch information
@drokeye
drokeye committed Jun 21, 2025
commit 6ab2c3c79779ac9c55e82de01deecc8154030a6e
5 changes: 1 addition & 4 deletions google/cloud/bigquery/dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,6 @@ def entity_id(self) -> Optional[Union[Dict[str, Any], str]]:
def __eq__(self, other):
if not isinstance(other, AccessEntry):
return NotImplemented

return (
self.role == other.role
and self.entity_type == other.entity_type
Expand Down Expand Up @@ -571,7 +570,6 @@ def to_api_repr(self):
resource[k] = v

return resource


@classmethod
def from_api_repr(cls, resource: dict) -> "AccessEntry":
Expand All @@ -585,7 +583,6 @@ def from_api_repr(cls, resource: dict) -> "AccessEntry":
google.cloud.bigquery.dataset.AccessEntry:
Access entry parsed from ``resource``.
"""

role = resource.get("role")
condition = None
if "condition" in resource:
Expand All @@ -595,7 +592,7 @@ def from_api_repr(cls, resource: dict) -> "AccessEntry":
entity_id = None

for key, value in resource.items():
Copy link
Copy Markdown
Contributor

@Linchin Linchin Jun 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to https://github.com/googleapis/python-bigquery/pull/2218/files#r2162644585, could you explain why we need to manually copy the resource dict? It doesn't seem very future-proof if we add any new fields in the future. Also, we deliberately used resource.copy() to make a shallow copy, mostly in order to reduce complexity and increase speed. Is deepcopy necessary here?
Copy link
Copy Markdown
Contributor Author

@drokeye drokeye Jun 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I originally switched to a manual copy/deepcopy mainly to help isolate issues during tests, especially with view/routine equality, where we needed full round-trip consistency via _properties. But I agree, it's my bad it's not ideal. Reverted this back to the original code.
if key in ("role", "condition"):
if key in ("role","condition"):
continue
entity_type = key
entity_id = value
Expand Down
61 changes: 50 additions & 11 deletions tests/unit/test_dataset.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1767,26 +1767,65 @@ def test__hash__with_minimal_inputs(self):
description=None,
)
assert hash(cond1) is not None

def test_access_entry_view_equality(self):

from google.cloud import bigquery

entry1 = bigquery.dataset.AccessEntry(
entity_type="view",
entity_id={
"projectId": "my_project",
"datasetId": "my_dataset",
"tableId": "my_table",
"projectId":"my_project",
"datasetId":"my_dataset",
"tableId":"my_table",
},
)
entry2 = bigquery.dataset.AccessEntry.from_api_repr({
"view": {
"projectId": "my_project",
"datasetId": "my_dataset",
"tableId": "my_table",
"view":{
"projectId":"my_project",
"datasetId":"my_dataset",
"tableId":"my_table",
}
})

assert entry1 == entry2
entry3 = bigquery.dataset.AccessEntry(
entity_type="routine",
entity_id={
"projectId":"my_project",
"datasetId":"my_dataset",
"routineId":"my_routine",
},
)

entry4 = bigquery.dataset.AccessEntry.from_api_repr({
"routine":{
"projectId":"my_project",
"datasetId":"my_dataset",
"routineId":"my_routine",
}
})

entry5 = bigquery.dataset.AccessEntry(
entity_type="dataset",
entity_id={
"dataset": {
"projectId":"my_project",
"datasetId":"my_dataset",
},
"target_types":"VIEWS",
},
)

entry6 = bigquery.dataset.AccessEntry.from_api_repr({
"dataset":{
"dataset":{
"projectId":"my_project",
"datasetId":"my_dataset",
},
"target_types":"VIEWS",
}
})

assert entry1 == entry2
assert entry3 == entry4
assert entry5 == entry6
Loading