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

shallow-copy DictDB query results #1556

Merged
merged 2 commits into from Apr 14, 2012
Merged

shallow-copy DictDB query results #1556

merged 2 commits into from Apr 14, 2012

Conversation

minrk
Copy link
Member

@minrk minrk commented Apr 6, 2012

db_query() with default keys pops buffers, and this would result in actually removing the buffers from the record itself. Later calls to get_result() would then raise a KeyError, which would go unhandled.

This PR adds shallow-copy to DictDB results, so that changes to the returned dict are not reflected in the db itself.

The bug revealed a general issue: ZMQStreams are closed when an exception is raised in their on_recv() callback (following the IOStream pattern, and on the general principal that a socket with failing handlers should not be allowed to continue), which meant that invoking this bug would prevent any future queries to the Hub. So in addition to fixing the particular bug, a decorator is added that catches and logs exceptions, and is applied to all on_recv() handlers in IPython.parallel, such that other similar bugs will no longer result in the closure of the socket.

Previously failing tests included at the db and client levels.

repro prior to fix:

from IPython import parallel
rc = parallel.Client()
rc2 = parallel.Client()
# ensure there is at least one result:
rc[:].apply_sync(lambda : 1)
# db_query with default keys results in pop()
rc.db_query({'msg_id' : {'$ne' : None}})
# get_result handler fails with KeyError, so the 2nd line below with .get() will hang:
c = rc2.get_result(rc.history[-1])
c.get()

pop/edit actions on the results of queries should not affect the records themselves.

Hub.db_query pops buffers out of results in some situations, which would break future result_requests.  

Associated tests are included.
Logs uncaught exceptions, rather than allowing them to raise, which would cause the socket to be closed, preventing further requests.

Reduces impact of bugs like the one fixed in the previous commit.
@fperez
Copy link
Member

fperez commented Apr 14, 2012

Great! Code looks clean and I confirmed both the prior bug and that the fix works. Glad to see the test coverage on this getting more robust. Thanks! Merging now.

fperez added a commit that referenced this pull request Apr 14, 2012
shallow-copy DictDB query results

`db_query()` with default keys pops buffers, and this would result in actually removing the buffers from the record itself.  Later calls to `get_result()` would then raise a KeyError, which would go unhandled.

This PR adds shallow-copy to DictDB results, so that changes to the returned dict are not reflected in the db itself.

The bug revealed a general issue: ZMQStreams are closed when an exception is raised in their `on_recv()` callback (following the IOStream pattern, and on the general principal that a socket with failing handlers should not be allowed to continue), which meant that invoking this bug would prevent any future queries to the Hub.  So in addition to fixing the particular bug, a decorator is added that catches and logs exceptions, and is applied to all `on_recv()` handlers in IPython.parallel, such that other similar bugs will no longer result in the closure of the socket.

Previously failing tests included at the db and client levels.
@fperez fperez merged commit 816e3fa into ipython:master Apr 14, 2012
mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014
shallow-copy DictDB query results

`db_query()` with default keys pops buffers, and this would result in actually removing the buffers from the record itself.  Later calls to `get_result()` would then raise a KeyError, which would go unhandled.

This PR adds shallow-copy to DictDB results, so that changes to the returned dict are not reflected in the db itself.

The bug revealed a general issue: ZMQStreams are closed when an exception is raised in their `on_recv()` callback (following the IOStream pattern, and on the general principal that a socket with failing handlers should not be allowed to continue), which meant that invoking this bug would prevent any future queries to the Hub.  So in addition to fixing the particular bug, a decorator is added that catches and logs exceptions, and is applied to all `on_recv()` handlers in IPython.parallel, such that other similar bugs will no longer result in the closure of the socket.

Previously failing tests included at the db and client levels.
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

2 participants