search

igorkasyanchuk / rails_performance

Monitor performance of you Rails applications (self-hosted and free)
https://www.railsjazz.com/
MIT License
957 stars 54 forks source link

Route mounting #12

Closed alagos closed 3 years ago

alagos commented 3 years ago

rails/performance looks like a good path, but most of the time people tend to use admin/something for all their administration stack. Indeed in my current work, there's a whole security system configured out of the rails scope, where any url starting with /admin is only accessible in the office or via VPN, so I made the changes to mount this engine into /admin/performance. Also, I updated the readme to show how to make this work with the manual mounting (besides I removed #Usage section and moved up #Installation, which it was pretty much the same info) and gave some basic info about how to authenticate with devise. Just in case, this is something that other rails engines also do, like rails_admin or pghero to mention a few of them, where the dev decides what's the route they want to.

alagos commented 3 years ago

Forgot to change the tests. Working on it...

alagos commented 3 years ago

All passing now

Finished in 0.360613s, 55.4611 runs/s, 105.3761 assertions/s.
20 runs, 38 assertions, 0 failures, 0 errors, 0 skips

BTW, for the TODO list maybe would be good to add the project to travis/circleci and automate the tests.

igorkasyanchuk commented 3 years ago

Hello @alagos

I don't like the idea of changing routes. I like /rails/performance prefix.

I think it's possible to change, you changing "as" option. Doesn't it work?

PS: Travis CI is already working https://travis-ci.org/github/igorkasyanchuk/rails_performance :)

igorkasyanchuk commented 3 years ago

Actually @alagos try now https://github.com/igorkasyanchuk/rails_performance/commit/04e995b6267c3c0c8be78b76a7873666a27084dc try to use version of the gem from master and see it works for you.

If yes - I'll release this today.

igorkasyanchuk commented 3 years ago

in config you need to specify config.mount_at = '/rails/performance'

not sure if it will work with devise

igorkasyanchuk commented 3 years ago

@alagos please confirm if it works

igorkasyanchuk commented 3 years ago

I think it should work now, just released a new version: rails_performance-0.9.6.gem

And I'd like to ask you to improve readme, I like what you did in your version, and maybe you can create a new PR with more instructions on how to use with devise, etc.

salzig commented 3 years ago

i really like the way this PR changes the rails integration. It would allow me to easily use rails authentication route wrapper, as shown in the readme, and thereby omit authentication handling duplication.

igorkasyanchuk commented 3 years ago

if you can fix the conflicts (make fork from the fork). But I don't like the idea of changing the default path in readme from /rails/performance to /admin/performance. It could be mentioned as an example but not as a default instruction. I just like prefix "rails"