Notebook query elapsed time #644
Conversation
pgml-dashboard/templates/cell.html
Outdated
| @@ -75,7 +75,14 @@ | |||
| <div class="notebook-duration"> | |||
| <div class="markdown-body"> | |||
| <div class="flex flex-row-reverse"> | |||
| <code>TODO</code> | |||
| <code>Time: <%- | |||
There was a problem hiding this comment.
Did you consider moving this inside the sql.html <div class="markdown-body">? I think that might be better to track each statement independently, but I only thought it through because of the warning about an unused variable in the template.
Otherwise, I'd move it to the cell template that uses it.
There was a problem hiding this comment.
Sorry I totally missed the unused variable error. I switched back to stable from nightly and can see it now.
That was my original plan, but then I saw in the models.rs the Cell struct already had an execution_time attribute and the cell.html had a TODO section for it.
I just pushed a commit that also shows the timing for each sql statement independently. Let me know what you think! Do you want me to remove the rendering of the Cell execution_time in the cell.html? If so, should I completely remove the execution_time attribute for the Cell struct and update the migrations to not include that column?
pgml-dashboard/templates/cell.html
Outdated
| microseconds @ 1000000.. => format!("{}s", (microseconds as f64) / 1000000.), | ||
| microseconds @ 1000..=999999 => format!("{}ms", (microseconds as f64) / 1000.), | ||
| microseconds @ 0..=999 => format!("{}μs", microseconds), | ||
| _ => "0".to_string(), |
There was a problem hiding this comment.
This is unreachable, and unnecessary since your conditions are exhaustive (according the the compiler).
pgml-dashboard/templates/sql.html
Outdated
| @@ -24,6 +24,15 @@ | |||
| <% } %> | |||
| </tbody> | |||
| </table> | |||
| <code> | |||
| Time: | |||
| <%- match execution_duration.as_micros() { | |||
There was a problem hiding this comment.
It'd be nice to have a util function to convert a duration to a string consistently throughout the app that can be reused across these placements.
There was a problem hiding this comment.
Added a utils.rs module with a function for doing this. I made it take an f64, as we convert it to that regardless of whether we are working with a std::time::Duration or postgres Interval.
pgml-dashboard/templates/sql.html
Outdated
| @@ -24,6 +24,15 @@ | |||
| <% } %> | |||
| </tbody> | |||
| </table> | |||
| <code> | |||
There was a problem hiding this comment.
It would be nice to hide this block if there is only a single statement, so that we don't double display the timing in that case.
Added support for dashboard notebooks showing server side time elapsed for queries