Avoid unnecessary grant call #791
Conversation
tests/unit/test_dataset.py
Outdated
| @@ -24,7 +24,7 @@ def test_add_access_entry_to_dataset_idempotently_adds_entries(): | |||
| dataset = add_access_entry_to_dataset(dataset, access_entry) | |||
| assert access_entry in dataset.access_entries | |||
| dataset = add_access_entry_to_dataset(dataset, access_entry) | |||
| assert len(dataset.access_entries) == 1 | |||
| assert dataset is None | |||
There was a problem hiding this comment.
I would argue this is not idempotent if running it once adds an attribute and running it twice deletes the object. Are you sure this is the right desired behavior?
There was a problem hiding this comment.
I should rename these tests because you're rightly calling out that this operation should no longer be considered idempotent.
There was a problem hiding this comment.
Ah ok. I looked at where the code is used. It looks like dataset is representing the change in access entries, not the dataset itself. Could we update the name of dataset, add_access_entry_to_dataset, or both, or make that more explicit somehow? It's not intuitive to me that add_access_entry_to_dataset(dataset, access_entry) would delete dataset if access_entry were already contained in dataset.access_entries.
* remove call to dataset update if dataset has not changed * add changie * fix unit test naming * seperate entry check from update --------- Co-authored-by: Mike Alfare <13974384+mikealfare@users.noreply.github.com> (cherry picked from commit 85efa2a)
* remove call to dataset update if dataset has not changed * add changie * fix unit test naming * seperate entry check from update --------- Co-authored-by: Mike Alfare <13974384+mikealfare@users.noreply.github.com> (cherry picked from commit 85efa2a) Co-authored-by: colin-rogers-dbt <111200756+colin-rogers-dbt@users.noreply.github.com>
resolves #770
Description
remove call to dataset update if dataset has not changed
Checklist
changie newto create a changelog entry