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

Delete returns None but expected a dict #1008

Closed
Garito opened this issue May 26, 2015 · 28 comments
Closed

Delete returns None but expected a dict #1008

Garito opened this issue May 26, 2015 · 28 comments
Assignees
Labels
Milestone

Comments

@Garito
Copy link

Garito commented May 26, 2015

I'm trying to delete a document with self.delete() but it raises and error:
File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/mongoengine/queryset/base.py", line 467, in delete
return result["n"]
TypeError: 'NoneType' object has no attribute 'getitem'

I've checked the result with print result and it is None
The deletion is made ok but the try to return result["n"] raises and error because result is None

Did I missed something or did I find a bug?

Thanks!

@MRigal
Copy link
Member

MRigal commented Jun 2, 2015

This is strange, we have test covering that... I'm wondering why pymongo returns None.
Which version of pymongo are you using?
This should anyway be fixed when using pymongo3 and using the new delete_one or delete_many methods.
I guess you have some corrupted data and/or are doing a remove on a queryset with invalid filtering with only() or exclude(), something a bit edge case.
If nobody else has the case, we won't treat it as an urgent bug, but let it be fixed by further better implementation of pymongo3+ possibilities

Thanks for reporting @Garito

@Garito
Copy link
Author

Garito commented Jun 2, 2015

Glad to help
Data:
pymongo=2.8
mongoengine=0.9.0
flask-mongoengine=0.7.1

I've tryed to install pymongo lastest version but it raises some errors (sorry for don't provide the traceback) perhaps will be this?

@MRigal
Copy link
Member

MRigal commented Jun 2, 2015

immm I have absolutely no idea of flask-mongoengine and what it could add...
Yes latest pymongo would require to use mongoengine from master to work. Keep 2.8 for your production work, but eventually try with master just for the case

@Garito
Copy link
Author

Garito commented Jun 2, 2015

Could it be the convination of pymongo 2.8 and mongodb 3.0?

@MRigal
Copy link
Member

MRigal commented Jun 2, 2015

It shouldn't, as it is quite common and I myself used that for a while,
without having failures there

2015-06-02 12:18 GMT+02:00 Garito notifications@github.com:

Could it be the convination of pymongo 2.8 and mongodb 3.0?


Reply to this email directly or view it on GitHub
#1008 (comment)
.

@Garito
Copy link
Author

Garito commented Jun 2, 2015

Is there anything I could do to provide you more info?
I would like to solve this because is super annoying not to have a clue about the issue and it prevents me to advance on my CRUD operations

Thanks!

@MRigal
Copy link
Member

MRigal commented Jun 3, 2015

@Garito I have really no idea why it is None at that point. It seems to be inside pymongo...
You may first try two things here:

  1. Test with pymongo 3.0.X and mongoengine master
  2. print not only result, but also queryset._query and write_concern, if something happens to be strange there
    Please answer here, but if it brings nothing, you may write on the mongodb-users list, as it would be pymongo

@Garito
Copy link
Author

Garito commented Jun 4, 2015

Before to try to install pymongo and so on I've tryed to print the queryset._query and write_concert in case this info as it is could be useful:
queryset._query => {'_id': ObjectId('55701184acffc705df7a33d4')}
write_concern => {}
result => None

Is this useful?

Thanks!

@Garito
Copy link
Author

Garito commented Jun 4, 2015

With pymongo=3.0.2 and -e git://github.com/mongoengine/mongoengine.git@03ed5c3#egg=mongoengine-master it raises another different error:
AttributeError: 'Cursor' object has no attribute '_Cursor__snapshot'
this time with any request so it is not possible to work with them

Any ideas?

@MRigal
Copy link
Member

MRigal commented Jun 4, 2015

Really weird it doesn't return anything on such a simple and straight-forward delete query... PyMongo or MongoDB internal problem !?!?

Regarding the second one, immm this is not possible... the actual master has the fix for _Cursor__snapshot...
Are you sure your virtualenv or python env is pointing to the right mongoengine?
Else maybe you would have detected a new problem, which would be very interesting for us.

Could you please paste the full stack trace for the AttributeError and eventually verify that the file raising the _Cursor__snapshot error is the same than in the repo? Also deleting .pyc files and so on before

@Garito
Copy link
Author

Garito commented Jun 4, 2015

Let me know if I've done the mongoengine's master branch install correctly, please:
pip install -U git+git://github.com/mongoengine/mongoengine@master
Since it said: Successfully installed mongoengine-0.9.0

@Garito
Copy link
Author

Garito commented Jun 4, 2015

Let me copy the whole log since I've started the flask app till the error. Remember that now this error is raised no matter what I ask for to Flask. The logs:

127.0.0.1 - - [04/Jun/2015 11:23:58] "GET /projects/contents HTTP/1.1" 500 -
Traceback (most recent call last):
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask/app.py", line 1836, in __call__
    return self.wsgi_app(environ, start_response)
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask/app.py", line 1820, in wsgi_app
    response = self.make_response(self.handle_exception(e))
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask/app.py", line 1403, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask/app.py", line 1817, in wsgi_app
    response = self.full_dispatch_request()
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask/app.py", line 1477, in full_dispatch_request
    rv = self.handle_user_exception(e)
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask/app.py", line 1381, in handle_user_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask/app.py", line 1473, in full_dispatch_request
    rv = self.preprocess_request()
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask/app.py", line 1666, in preprocess_request
    rv = func()
  File "/Users/garito/TimeFounder/App/0.3/TimeFounder/__init__.py", line 105, in before_request
    g.locale = get_locale()
  File "/Users/garito/TimeFounder/App/0.3/TimeFounder/__init__.py", line 94, in get_locale
    if current_user.is_anonymous():
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/werkzeug/local.py", line 338, in __getattr__
    return getattr(self._get_current_object(), name)
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/werkzeug/local.py", line 297, in _get_current_object
    return self.__local()
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask_login.py", line 46, in <lambda>
    current_user = LocalProxy(lambda: _get_user())
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask_login.py", line 794, in _get_user
    current_app.login_manager._load_user()
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask_login.py", line 363, in _load_user
    return self.reload_user()
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask_login.py", line 325, in reload_user
    user = self.user_callback(user_id)
  File "/Users/garito/TimeFounder/App/0.3/TimeFounder/__init__.py", line 66, in load_user
    return app.getClass("User").objects(id=userid).first()
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/mongoengine/queryset/base.py", line 314, in first
    result = queryset[0]
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/mongoengine/queryset/base.py", line 164, in __getitem__
    return queryset._document._from_son(queryset._cursor[key],
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/pymongo/cursor.py", line 531, in __getitem__
    for doc in clone:
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/pymongo/cursor.py", line 983, in next
    if len(self.__data) or self._refresh():
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask_mongoengine/operation_tracker.py", line 168, in _cursor_refresh
    snapshot = privar("snapshot")
  File "/Users/garito/TimeFounder/App/0.3/env/lib/python2.7/site-packages/flask_mongoengine/operation_tracker.py", line 136, in privar
    return getattr(cursor_self, '_Cursor__{0}'.format(name))
AttributeError: 'Cursor' object has no attribute '_Cursor__snapshot'

@MRigal
Copy link
Member

MRigal commented Jun 4, 2015

OK, yep, the flask-mongoengine app is not that far and does not support PyMongo3 for now. It is not related or due to mongoengine itself... That's a pity, it would have been cool to test it, we may have had at least a more specific output. Sorry, I didn't knew about that.

I really don't know what to recommend you to do, I see two possibilities:

  1. Try to reproduce the query sent via flask to mongoengine to pymongo by something directly in pymongo to be able to report it to the mongoldb-user group, you may have found an edge-case bug in pymongo or MongoDB itself, even if only on the pymongo 2.8 "old" branch.
  2. Make yourself or find somebody to make flask-mongoengine be pymongo3 compatible by doing similar changes as the ones contained in the PR Pymongo3 compatibility #946 on mongoengine side. Even if this won't guarantee a positive result...

@Garito
Copy link
Author

Garito commented Jun 4, 2015

So to recap:
This could be Flask-Mongoengine, pymongo or MongoDB itself, is this correct?
I have two options:
Try to make Flask-Mongoengine compatible with version 3 of pymongo/mongodb
Try to find the bug on pymongo or MongoDB

am I correct?

@MRigal
Copy link
Member

MRigal commented Jun 4, 2015

Almost ;-)
It could be pymongo or MongoDB itself, not flask-mongoengine. But:

  1. It could be an edge-case of communication between pymongo 2.X and mongoDB 3.x. In this case, an upgrade to pymongo 3.X may already fix it, but to use it, you would need to have a flask-mongoengine compatible with Pymongo3
  2. It could be a problem deeper inside PyMongo, MongoDB or your specific storage/setting and you could try to find the bug, but probably only with the help of people from MongoDB Inc :-)

@Garito
Copy link
Author

Garito commented Jun 4, 2015

I see
What do you suggest that will be my best fist attempt? flask-mongoengine or pymongo/mongodb? or both at the same time?

Sorry for asking too much on this point but I'm scared to death on this task since seems to me a very dark case which everyone will defend themselves before admit it (since is an edge case)

Do you recommend a protocol to reduce the friction or something?

@MRigal
Copy link
Member

MRigal commented Jun 4, 2015

Indeed tough questions :-)

I would do both in parallel:

  1. Try to reproduce the query sent via flask to mongoengine to pymongo by something directly in pymongo to be able to report it to the mongoldb-user group, it will take some time to get an answer
  2. In the meanwhile, I think fixing flask-mongoengine should not take more than a couple hours. Even if I don't want in theory to put effort in this separate project, I would volunteer to review and fine-tune the PR you could submit for this particular point. It may give another clue or clarify the responsibility of PyMongo 2.X, which could help to get an answer at point 1.

This is how I see minimal friction, but still not trivial

@MRigal
Copy link
Member

MRigal commented Jun 4, 2015

Whenever you think it doesn't concern anymore mongoengine, like after or in case I don't have to review the PR for flask-mongoengine, also please close the ticket, that you may reopen if you have newer information or output from MongoDB Inc...

@Garito
Copy link
Author

Garito commented Jun 9, 2015

Hi!
I was looking for this issue and I end up looking for the official documentation to find out the issue is in the mongoengine itself. From the official pymongo 2.8 documentation (http://api.mongodb.org/python/2.8/api/pymongo/collection.html#pymongo.collection.Collection.remove):

Returns:

A document (dict) describing the effect of the remove or None if write acknowledgement is disabled.

So it is not a bug that pymongo's collection.remove returns None if the write acknowledgement is disable but mongoengine doesn't have this case into account so I have to conclude that the bug is on mongoengine

What do you think?

@Garito
Copy link
Author

Garito commented Jun 9, 2015

BTW the change to solve this issue will be to update line 464 of queryset/base.py from

return result["n"]

to

return result["n"] if result and "n" in result else result

I think this update will be correct giving the fact that if write_concern is empty or 0 it's ok to return None so result will be None if the write_concern is 0 or {} (which is my actual case)

I've made this modification in my installed library to test and it works ok for me

@Garito
Copy link
Author

Garito commented Jun 13, 2015

Hi!
I really need this correction
Please could someone give me some feedback about how to include it to the master?

Thanks!

@thedrow
Copy link
Contributor

thedrow commented Jun 13, 2015

Ok so what we need you to do right now it to supply a test case that attempts to delete an object while specifying a write concern.
The test case should initially fail and then pass with the fix you suggested above.
It does make sense that your suggested fix will resolve this issue but we need to verify that it does so for both PyMongo 2.7 and 3.x.

@MRigal MRigal added this to the 0.10 milestone Jun 14, 2015
@MRigal
Copy link
Member

MRigal commented Jun 14, 2015

Yes, please write a test with your diff and submit it as a PullRequest. If it is your first time, github has some tutorials on how to do it. It is quite easy, you'll see. There is also a contributing doc in mongoengine repository.

One thing, for your change, the following is enough:

return result.get("n") if result

as None is the default return value in python.

If you could do it in the next 7 days, it would even make it it into the next big release, the 0.10

@Garito
Copy link
Author

Garito commented Jun 14, 2015

Will try
I use to have problems to plan the tests but I will do my best! If you have a structure of the test in mind, please feel free to put them here I will try to fill the gaps

@MRigal
Copy link
Member

MRigal commented Jun 21, 2015

Thanks @Garito for reporting this real bug! I've written the PR #1037 to be able to include it into 0.10

@Garito
Copy link
Author

Garito commented Jun 21, 2015

Thank to you @MRigal! Nice to be useful
is it time to close this issue then?

@thedrow
Copy link
Contributor

thedrow commented Jun 21, 2015

@Garito When the bug fix will be merged.

@Garito
Copy link
Author

Garito commented Jun 22, 2015

@thedrow as @MRigal said above, it will be merged into 0.10 but, meanwhile you could path your library if you can't wait (look at the pull request in this threat to see what changes will be made for this particular issue)

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

No branches or pull requests

3 participants