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.

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

no-op on repeated calls to destroy #182

Merged
merged 1 commit into from
Jan 4, 2017
Merged

no-op on repeated calls to destroy #182

merged 1 commit into from
Jan 4, 2017

Conversation

msaffitz
Copy link
Contributor

@msaffitz msaffitz commented Jan 4, 2017

Calling destroy multiple times on the same document attempts to track each deletion, which ultimately fails with a RuntimeError when updating the (frozen) attributes hash with the incremented version number. (This is especially problematic / difficult to track down for documents with cyclic relations and cascading destroys).

This no-ops tracking of destroy for documents that are already destroyed.

@dblock
Copy link
Collaborator

dblock commented Jan 4, 2017

I've fixed the build in #183, rebase this please. Your changelog line needs to start with a capital letter too to make danger-changelog happy. Sorry about the hassle.

Calling destroy multiple times on the same document attempts to track each deletion, which ultimately fails with a RuntimeError when updating the (frozen) attributes hash with the incremented version number. (This is especially problematic / difficult to track down for documents with cyclic relations and cascading destroys).

This no-ops tracking of destroy for documents that are already destroyed.
@coveralls
Copy link

coveralls commented Jan 4, 2017

Coverage Status

Coverage increased (+0.0003%) to 99.749% when pulling 4cdd1de on apptentive-engineering:noop_repeat_destroys into ccc733e on mongoid:master.

2 similar comments
@coveralls
Copy link

Coverage Status

Coverage increased (+0.0003%) to 99.749% when pulling 4cdd1de on apptentive-engineering:noop_repeat_destroys into ccc733e on mongoid:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.0003%) to 99.749% when pulling 4cdd1de on apptentive-engineering:noop_repeat_destroys into ccc733e on mongoid:master.

@msaffitz
Copy link
Contributor Author

msaffitz commented Jan 4, 2017

@dblock no worries; rebased and Changelog fixed. Thanks for considering / merging.

@dblock dblock merged commit 7cc6ddf into mongoid:master Jan 4, 2017
@dblock
Copy link
Collaborator

dblock commented Jan 4, 2017

Merged, thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants