Implement elapsed_text display for request/response #48
Conversation
ghost
marked this pull request as ready for review
| @@ -27,6 +27,7 @@ class Breeze::Requests::ShowPage < Breeze::BreezeLayout | |||
| mount Breeze::DescriptionListRow, "Action", req.action | |||
| req.breeze_response.try do |resp| | |||
| mount Breeze::DescriptionListRow, "Response Status", "#{resp.status} #{Wordsmith::Inflector.humanize(HTTP::Status.from_value?(resp.status))}" | |||
| mount Breeze::DescriptionListRow, "Response Elapsed Time", resp.elapsed_text | |||
There was a problem hiding this comment.
I didn't find any test where I could check if this display is correct / present
| @@ -65,18 +65,21 @@ module Breeze::ActionHelpers | |||
| ) | |||
| Lucky::Log.dexter.debug { {debug_at: Breeze::Requests::Show.url(req.id)} } | |||
| Fiber.current.breeze_request = req | |||
| Fiber.current.breeze_request_start = Time.monotonic | |||
There was a problem hiding this comment.
Figured this was easier than storing the start in the db.
|
Hey @Liberatys thanks for contributing to this! So, there's a few things we need to think out first before considering this. The first thing is how migrations are added in afterwards. (See #47). The next thing is that the current action timing actually happens in the I didn't do a great job of explaining what needed to be done, so sorry for that. Looking at your PR, I think it might be ok, but I'd like to consider if there's anything we may have to add in to Lucky itself in order to track this timing better. I will do some digging and then come back with some notes on the PR and we can go from there. Thanks! |
|
Just letting you know I haven't forgotten about this. Just had to wrap up some other stuff. That first issue I mentioned about the migrations is moot... Ok, so for storing the timing, we're going to have to hook in to Lucky for this. I think it might be a hybrid solution of what you have here with the database storage, and hooking in to the LogHandler. My guess is we can use Pulsar to publish the duration, and then have Breeze subscribe to that event and store it in the DB, then it's just a matter of displaying that. I also want to try and setup some sort of benchmark to test that adding the pulsar in to the loghandler doesn't affect application performance any. I'll probably work on that part, then come back to this and we can update to use that new stuff. |
|
Ok, this took longer than I wanted, but this PR is now possible to do. It'll require a bit of rework though. luckyframework/lucky#1601 Basically, this just needs to subscribe to |
ghost
closed this by deleting the head repository
Adds the elapsed_text field to the breeze_response model.
Should address the feature request in #33.