hammerlab / cycledash

Variant Caller Analysis Dashboard and Data Management System
Other
35 stars 2 forks source link

Fix VCF download feature #818

Closed armish closed 9 years ago

armish commented 9 years ago

See #738 for more context.

When clicked on the download, we were getting this error logged:

----------------------------------------
Exception happened during processing of request from ('127.0.0.1', 59979)
Traceback (most recent call last):
  File "/usr/local/Cellar/python/2.7.10/Frameworks/Python.framework/Versions/2.7/lib/python2.7/SocketServer.py", line 295, in _handle_request_noblock
    self.process_request(request, client_address)
  File "/usr/local/Cellar/python/2.7.10/Frameworks/Python.framework/Versions/2.7/lib/python2.7/SocketServer.py", line 321, in process_request
    self.finish_request(request, client_address)
  File "/usr/local/Cellar/python/2.7.10/Frameworks/Python.framework/Versions/2.7/lib/python2.7/SocketServer.py", line 334, in finish_request
    self.RequestHandlerClass(request, client_address, self)
  File "/usr/local/Cellar/python/2.7.10/Frameworks/Python.framework/Versions/2.7/lib/python2.7/SocketServer.py", line 657, in __init__
    self.finish()
  File "/usr/local/Cellar/python/2.7.10/Frameworks/Python.framework/Versions/2.7/lib/python2.7/SocketServer.py", line 716, in finish
    self.wfile.close()
  File "/usr/local/Cellar/python/2.7.10/Frameworks/Python.framework/Versions/2.7/lib/python2.7/socket.py", line 283, in close
    self.flush()
  File "/usr/local/Cellar/python/2.7.10/Frameworks/Python.framework/Versions/2.7/lib/python2.7/socket.py", line 307, in flush
    self._sock.sendall(view[write_offset:write_offset+buffer_size])
error: [Errno 32] Broken pipe
----------------------------------------

which was stemming from a simple coding mistake.

Review on Reviewable

hammer commented 9 years ago

is there a test you could add that would have caught this mistake?

armish commented 9 years ago

yeah, it should do be doable using one of the test files we have. Let me add that and update the PR.

armish commented 9 years ago

Adding a download test was much harder than I expected (due to some magic happening with authentication), but this is done now. On the bright side: we now have support for basic authentication for all pages within Cycledash.

ihodes commented 9 years ago

Looks great! One minor comment.

ihodes commented 9 years ago

Looks good!

armish commented 9 years ago

Thanks!