Change SQLAlchemy utility views to access the query internally. #227
Conversation
| @@ -156,6 +156,7 @@ def sget(opt, default=None): | |||
| exc_history.eval_exc = intercept_exc == 'debug' | |||
|
|
|||
| def toolbar_tween(request): | |||
| request.pdbt_id = hexlify(id(request)) | |||
There was a problem hiding this comment.
I think this probably belongs lower. Probably just before where the toolbar is instantiated.
| config.add_route('debugtoolbar.sql_select', '/sqlalchemy/sql_select') | ||
| config.add_route('debugtoolbar.sql_explain', '/sqlalchemy/sql_explain') | ||
| config.add_route('debugtoolbar.sql_select', '/{request_id}/sqlalchemy/sql_select') | ||
| config.add_route('debugtoolbar.sql_explain', '/{request_id}/sqlalchemy/sql_explain') |
There was a problem hiding this comment.
I think we should keep the addon namespacing at the root of the url to avoid any weird ordering problems in the future. Considering moving the request_id to the end or middle or even as a query string arg next to the query_index?
There was a problem hiding this comment.
Meh, I guess we already have /{request_id}... I guess I'll retract that request.
There was a problem hiding this comment.
I could do /sqlalchemy/{request_id}/sql_select/{query_index}, if you like.
Given that both parameters are required to be able to do anything, I think they both belong in the route.
There was a problem hiding this comment.
Ok let's formalize this and try to keep consistent.
/{request_id}/<panel_name>/...
So in this case it'd be /{request_id}/sqlalchemy/select/{query_index}.
So long as no one does /{request_id}/{another_placeholder} we won't have ambiguous routes that depend on panel include order.
This way the params don't have to be JSON-serializable.
|
@mmerickel I believe I've resolved the routes to your satisfaction. |
Change SQLAlchemy utility views to access the query internally.
|
Excellent work, thank you. |
This allows SELECT/EXPLAIN to be run for queries which have params that are not JSON-serializable.
Also, it's arguably a cleaner design.
Closes #222.