Skip to content

mark_as_changed issue #927

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
Merged
thedrow merged 1 commit into from
Apr 10, 2015
Merged

mark_as_changed issue #927

thedrow merged 1 commit into from
Apr 10, 2015

Conversation

@Catstyle
Copy link
Contributor

@Catstyle Catstyle commented Apr 1, 2015

fix mark_as_changed: rm lower level changed fields otherwise may cause conflict update error
this issue appears when retrieve from db

In [34]: class Foo(mongoengine.Document):

    bar = mongoengine.DictField()
   ....:     

In [35]: Foo.objects.delete()
Out[35]: 1

In [36]: foo = Foo()

In [37]: foo.save()
Out[37]: <Foo: Foo object>

In [38]: foo = Foo.objects.first()

In [39]: foo.bar['what'] = 'ever'

In [40]: foo.bar['other'] = {}

In [41]: foo._changed_fields
Out[41]: ['bar.what', 'bar.other']

In [42]: foo.bar['other']['what'] = 'ever'

In [43]: foo._changed_fields
Out[43]: ['bar.what', 'bar.other', 'bar.other.what']

In [44]: foo.save()
---------------------------------------------------------------------------
OperationError                            Traceback (most recent call last)
<ipython-input-44-e6645f54a3c0> in <module>()
----> 1 foo.save()

.../mongoengine/document.pyc in save(self, force_insert, validate, clean, write_concern, cascade, cascade_kwargs, _refs, save_condition, **kwargs)
    365                 message = u'Tried to save duplicate unique keys (%s)'
    366                 raise NotUniqueError(message % unicode(err))
--> 367             raise OperationError(message % unicode(err))
    368         id_field = self._meta['id_field']
    369         if created or id_field not in self._meta.get('shard_key', []):

OperationError: Could not save document (Cannot update 'bar.other' and 'bar.other.what' at the same time)

Review on Reviewable

@landscape-bot
Copy link

landscape-bot commented Apr 1, 2015

Code Health
Repository health increased by 0.11% when pulling f81ecb8 on Catstyle:feature/mark_as_changed_issue into cbd2a44 on MongoEngine:master.

@coveralls
Copy link

coveralls commented Apr 1, 2015

Coverage Status

Changes Unknown when pulling f81ecb8 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

1 similar comment
@coveralls
Copy link

coveralls commented Apr 1, 2015

Coverage Status

Changes Unknown when pulling f81ecb8 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@Catstyle
Copy link
Contributor Author

Catstyle commented Apr 1, 2015

sure, just changed to a forloop

@landscape-bot
Copy link

landscape-bot commented Apr 1, 2015

Code Health
Repository health increased by 0.13% when pulling 649e631 on Catstyle:feature/mark_as_changed_issue into cbd2a44 on MongoEngine:master.

@coveralls
Copy link

coveralls commented Apr 1, 2015

Coverage Status

Changes Unknown when pulling 649e631 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

6 similar comments
@coveralls
Copy link

coveralls commented Apr 1, 2015

Coverage Status

Changes Unknown when pulling 649e631 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 1, 2015

Coverage Status

Changes Unknown when pulling 649e631 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 1, 2015

Coverage Status

Changes Unknown when pulling 649e631 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 1, 2015

Coverage Status

Changes Unknown when pulling 649e631 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 1, 2015

Coverage Status

Changes Unknown when pulling 649e631 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 1, 2015

Coverage Status

Changes Unknown when pulling 649e631 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@Catstyle Catstyle mentioned this pull request Apr 1, 2015
Closed
syndbg
syndbg reviewed Apr 1, 2015
View reviewed changes
mongoengine/base/document.py Outdated
Copy link

@syndbg syndbg Apr 1, 2015

Choose a reason for hiding this comment

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

What's your idea behind the copy of self._changed_fields and why would you use non-copied self._changed_fields's remove with argument - copied field?

Copy link
Contributor Author

@Catstyle Catstyle Apr 2, 2015

Choose a reason for hiding this comment

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

yeah, i thought it wrong, i mix it up with the dict that cannot change the instance when iterating

@landscape-bot
Copy link

landscape-bot commented Apr 2, 2015

Code Health
Repository health increased by 0.13% when pulling 02b095a on Catstyle:feature/mark_as_changed_issue into cbd2a44 on MongoEngine:master.

@coveralls
Copy link

coveralls commented Apr 2, 2015

Coverage Status

Changes Unknown when pulling 02b095a on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

7 similar comments
@coveralls
Copy link

coveralls commented Apr 2, 2015

Coverage Status

Changes Unknown when pulling 02b095a on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 2, 2015

Coverage Status

Changes Unknown when pulling 02b095a on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 2, 2015

Coverage Status

Changes Unknown when pulling 02b095a on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 2, 2015

Coverage Status

Changes Unknown when pulling 02b095a on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 2, 2015

Coverage Status

Changes Unknown when pulling 02b095a on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 2, 2015

Coverage Status

Changes Unknown when pulling 02b095a on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 2, 2015

Coverage Status

Changes Unknown when pulling 02b095a on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@MRigal
Copy link
Member

MRigal commented Apr 3, 2015

Great ! Add two small tests and it would be perfect (one for checking it get's well removed for a higher level change, and another to not add a lower level change)

@landscape-bot
Copy link

landscape-bot commented Apr 8, 2015

Code Health
Repository health increased by 0.16% when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into cbd2a44 on MongoEngine:master.

1 similar comment
@landscape-bot
Copy link

landscape-bot commented Apr 8, 2015

Code Health
Repository health increased by 0.16% when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into cbd2a44 on MongoEngine:master.

@coveralls
Copy link

coveralls commented Apr 8, 2015

Coverage Status

Changes Unknown when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

3 similar comments
@coveralls
Copy link

coveralls commented Apr 8, 2015

Coverage Status

Changes Unknown when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 8, 2015

Coverage Status

Changes Unknown when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 8, 2015

Coverage Status

Changes Unknown when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 8, 2015

Coverage Status

Changes Unknown when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

6 similar comments
@coveralls
Copy link

coveralls commented Apr 8, 2015

Coverage Status

Changes Unknown when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 8, 2015

Coverage Status

Changes Unknown when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 8, 2015

Coverage Status

Changes Unknown when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 8, 2015

Coverage Status

Changes Unknown when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 8, 2015

Coverage Status

Changes Unknown when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@coveralls
Copy link

coveralls commented Apr 8, 2015

Coverage Status

Changes Unknown when pulling c1dd7b7 on Catstyle:feature/mark_as_changed_issue into * on MongoEngine:master*.

@landscape-bot
Copy link

landscape-bot commented Apr 9, 2015

Code Health
Repository health increased by 0.16% when pulling b150053 on Catstyle:feature/mark_as_changed_issue into cbd2a44 on MongoEngine:master.

@landscape-bot
Copy link

landscape-bot commented Apr 9, 2015

Code Health
Repository health increased by 0.03% when pulling 8c5ae88 on Catstyle:feature/mark_as_changed_issue into 103a287 on MongoEngine:master.

@coveralls
Copy link

coveralls commented Apr 9, 2015 •
edited
Loading

Coverage Status

Coverage increased (+0.4%) to 73.414% when pulling 8c5ae88 on Catstyle:feature/mark_as_changed_issue into 103a287 on MongoEngine:master.

@Catstyle
Copy link
Contributor Author

Catstyle commented Apr 10, 2015

seems i have mixed up several branches in my own repo
should i close this PR and checkout from origin then open a new one?

@Catstyle
Copy link
Contributor Author

Catstyle commented Apr 10, 2015

i have checked out from origin master and force update the branch

@landscape-bot
Copy link

landscape-bot commented Apr 10, 2015

Code Health
Code quality remained the same when pulling 1756596 on Catstyle:feature/mark_as_changed_issue into 8fea2b0 on MongoEngine:master.

@coveralls
Copy link

coveralls commented Apr 10, 2015

Coverage Status

Coverage decreased (-0.01%) to 73.03% when pulling 1756596 on Catstyle:feature/mark_as_changed_issue into 8fea2b0 on MongoEngine:master.

6 similar comments
@coveralls
Copy link

coveralls commented Apr 10, 2015

Coverage Status

Coverage decreased (-0.01%) to 73.03% when pulling 1756596 on Catstyle:feature/mark_as_changed_issue into 8fea2b0 on MongoEngine:master.

@coveralls
Copy link

coveralls commented Apr 10, 2015

Coverage Status

Coverage decreased (-0.01%) to 73.03% when pulling 1756596 on Catstyle:feature/mark_as_changed_issue into 8fea2b0 on MongoEngine:master.

@coveralls
Copy link

coveralls commented Apr 10, 2015

Coverage Status

Coverage decreased (-0.01%) to 73.03% when pulling 1756596 on Catstyle:feature/mark_as_changed_issue into 8fea2b0 on MongoEngine:master.

@coveralls
Copy link

coveralls commented Apr 10, 2015

Coverage Status

Coverage decreased (-0.01%) to 73.03% when pulling 1756596 on Catstyle:feature/mark_as_changed_issue into 8fea2b0 on MongoEngine:master.

@coveralls
Copy link

coveralls commented Apr 10, 2015

Coverage Status

Coverage decreased (-0.01%) to 73.03% when pulling 1756596 on Catstyle:feature/mark_as_changed_issue into 8fea2b0 on MongoEngine:master.

@coveralls
Copy link

coveralls commented Apr 10, 2015

Coverage Status

Coverage decreased (-0.01%) to 73.03% when pulling 1756596 on Catstyle:feature/mark_as_changed_issue into 8fea2b0 on MongoEngine:master.

@thedrow thedrow merged commit 19dc312 into MongoEngine:master Apr 10, 2015
@MRigal
Copy link
Member

MRigal commented Apr 10, 2015

Thanks @Catstyle !

This was referenced May 19, 2015
Closed
Merged
@Catstyle Catstyle deleted the feature/mark_as_changed_issue branch June 30, 2015 03:36
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

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@Catstyle @landscape-bot @coveralls @MRigal @syndbg