[fix] Update watch command execution time to be correct for all iterations (#763) #1428
Conversation
|
@rolandwalker Let me know if this looks better. I was hoping to not have to include it in the iterator, but I do not see another way around that unless we get rid of the iterator bit to begin with. Could potentially just next(res) it each time and then use the dataclass without iteration (i.e. no for loop at all), but not sure if that is any less hacky or would even work. So if you know a better way to do that let me know |
| set_pager_enabled(False) | ||
| for sql, title in sql_list: | ||
| cur.execute(sql) | ||
| command = { |
There was a problem hiding this comment.
Why a dict instead of two new properties?
There was a problem hiding this comment.
Didn't want to add a bunch of random properties (presuming other commands will need other args), so figured a general command property with the dict that could be added to as needed would be better. But if you prefer having standalone properties for it all I can do that.
There was a problem hiding this comment.
Your implementation, your call.
There was a problem hiding this comment.
Okay I'll keep it as-is then. Don't want to make too many assumptions about future use since this is the first use case, and it can be adjusted later if we find another approach makes more sense given new use cases. Current imagined use case is other commands needing special args, and it fits that anyway.
2 participants
Description
Currently the watch command execution time is wrong for the 2nd+ iteration as it includes the execution time plus the watch command time. This PR fixes that issue so the correct execution time is reported for all watch iterations.
Fixes #763
Checklist
changelog.md.AUTHORSfile (or it's already there).uv run ruff check && uv run ruff format && uv run mypy --install-types .to lint and format the code.