Uh oh!
There was an error while loading. Please reload this page.
fix: make AccessEntry equality consistent with from_api_repr - #2218
Conversation
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
14bd1f0 to
969f09fCompare| def _normalize_entity_id(value): | ||
| """Ensure consistent equality for dicts like 'view'.""" | ||
| if isinstance(value, dict): | ||
| return dict(sorted(value.items())) |
There was a problem hiding this comment.
The normalization here may not work if there's nested dictionaries, and it might happen, for example here. The most reliable way might be using json.dumps() with sort_keys=True.
There was a problem hiding this comment.
Hey @Linchin — thanks for pointing that out.
You're absolutely right that dict(sorted(...)) won't handle deeply nested dictionaries reliably. In this case though, I tested equality on AccessEntry objects with view, routine, and dataset entity types — including nested structures like the one in dataset. The current approach passed all assertions in the test, and it seems to hold up.
Totally agree that json.dumps(..., sort_keys=True) would be the most robust and reliable method. I will change it right now👍
For reference, here’s the test case I used:
deftest_access_entry_view_equality(self):
fromgoogle.cloudimportbigqueryentry1=bigquery.dataset.AccessEntry(
entity_type="view",
entity_id={
"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",
}
})
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",
}
})
entry7=bigquery.dataset.AccessEntry(
entity_type="dataset",
entity_id={ "dataset": {
"location": {
"region": {
"zone": "asia-south1-a",
"subzone": "primary"
},
"country": "IN"
},
"datasetId": "my_dataset"
},
"target_types": "VIEWS"
},
)
entry8=bigquery.dataset.AccessEntry.from_api_repr({
"dataset": {
"target_types": "VIEWS",
"dataset": {
"datasetId": "my_dataset",
"location": {
"country": "IN",
"region": {
"subzone": "primary",
"zone": "asia-south1-a"
}
}
}
}
})
assertentry1==entry2assertentry3==entry4assertentry5==entry6assertentry7==entry8There was a problem hiding this comment.
Thanks for making the change, I guess all test cases passed because we didn't juggle the order of items in the nested dictionary :)
Linchin
commented
Jun 20, 2025
The lint test is failing, could you run |
drokeye
commented
Jun 21, 2025
Fixed it, should be fine now! |
Linchin
commented
Jun 23, 2025
I think there's more linting we need to fix |


Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes#2217 🦕
🐛 Bug Fix: AccessEntry equality for "view" entity
This PR fixes a bug where two
AccessEntryinstances representing the same view would not compare as equal:✅ Summary of Changes
__eq__method inAccessEntryto correctly handleentity_type="view"by normalizing the nested dictionary inentity_id.test_access_entry_view_equalityintests/unit/test_dataset.pyto assert correctness of equality logic for views.from_api_reprconsistency.