Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

compatibility with peek-mysql2 #451

Merged
merged 2 commits into from
Jan 25, 2021

Conversation

glaszig
Copy link
Contributor

@glaszig glaszig commented Jun 17, 2020

don't use alias_method since peek-mysql2 uses prepend on rails >= 5.
related to #437 and #444.

peek-mysql2 uses alias_method for rails < 5 though i opted not to distinguish as prepend was introduced with ruby 2.0 and this patch should work even if peek-mysql2 on rails 4 uses alias_method. but i might be wrong. let me know.

before this patch i've got this beautiful thing:

SystemStackError (stack level too deep):

peek-mysql2 (1.2.0) lib/peek-mysql2/timing.rb:8:in `ensure in query'
peek-mysql2 (1.2.0) lib/peek-mysql2/timing.rb:10:in `query'
rack-mini-profiler (2.0.0) lib/patches/db/mysql2.rb:25:in `block in query'
rack-mini-profiler (2.0.0) lib/patches/sql_patches.rb:12:in `record_sql'
rack-mini-profiler (2.0.0) lib/patches/db/mysql2.rb:24:in `query'

[...]

rack-mini-profiler (2.0.0) lib/patches/db/mysql2.rb:25:in `block in query'
rack-mini-profiler (2.0.0) lib/patches/sql_patches.rb:12:in `record_sql'
rack-mini-profiler (2.0.0) lib/patches/db/mysql2.rb:24:in `query'
peek-mysql2 (1.2.0) lib/peek-mysql2/timing.rb:6:in `query'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract_mysql_adapter.rb:201:in `block (2 levels) in execute'
activesupport (6.0.3.1) lib/active_support/dependencies/interlock.rb:48:in `block in permit_concurrent_loads'
activesupport (6.0.3.1) lib/active_support/concurrency/share_lock.rb:187:in `yield_shares'
activesupport (6.0.3.1) lib/active_support/dependencies/interlock.rb:47:in `permit_concurrent_loads'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract_mysql_adapter.rb:200:in `block in execute'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract_adapter.rb:722:in `block (2 levels) in log'
activesupport (6.0.3.1) lib/active_support/concurrency/load_interlock_aware_monitor.rb:26:in `block (2 levels) in synchronize'
activesupport (6.0.3.1) lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `handle_interrupt'
activesupport (6.0.3.1) lib/active_support/concurrency/load_interlock_aware_monitor.rb:25:in `block in synchronize'
activesupport (6.0.3.1) lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `handle_interrupt'
activesupport (6.0.3.1) lib/active_support/concurrency/load_interlock_aware_monitor.rb:21:in `synchronize'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract_adapter.rb:721:in `block in log'
activesupport (6.0.3.1) lib/active_support/notifications/instrumenter.rb:24:in `instrument'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract_adapter.rb:712:in `log'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract_mysql_adapter.rb:199:in `execute'
activerecord (6.0.3.1) lib/active_record/connection_adapters/mysql/database_statements.rb:41:in `execute'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract_mysql_adapter.rb:742:in `configure_connection'
activerecord (6.0.3.1) lib/active_record/connection_adapters/mysql2_adapter.rb:133:in `configure_connection'
activerecord (6.0.3.1) lib/active_record/connection_adapters/mysql2_adapter.rb:44:in `initialize'
activerecord (6.0.3.1) lib/active_record/connection_adapters/mysql2_adapter.rb:25:in `new'
activerecord (6.0.3.1) lib/active_record/connection_adapters/mysql2_adapter.rb:25:in `mysql2_connection'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract/connection_pool.rb:887:in `new_connection'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract/connection_pool.rb:931:in `checkout_new_connection'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract/connection_pool.rb:910:in `try_to_checkout_new_connection'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract/connection_pool.rb:871:in `acquire_connection'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract/connection_pool.rb:593:in `checkout'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract/connection_pool.rb:437:in `connection'
activerecord (6.0.3.1) lib/active_record/connection_adapters/abstract/connection_pool.rb:1119:in `retrieve_connection'
activerecord (6.0.3.1) lib/active_record/connection_handling.rb:221:in `retrieve_connection'
activerecord (6.0.3.1) lib/active_record/connection_handling.rb:189:in `connection'
activerecord (6.0.3.1) lib/active_record/migration.rb:562:in `call'
actionpack (6.0.3.1) lib/action_dispatch/middleware/callbacks.rb:27:in `block in call'
activesupport (6.0.3.1) lib/active_support/callbacks.rb:101:in `run_callbacks'
actionpack (6.0.3.1) lib/action_dispatch/middleware/callbacks.rb:26:in `call'
actionpack (6.0.3.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (6.0.3.1) lib/action_dispatch/middleware/actionable_exceptions.rb:17:in `call'
actionpack (6.0.3.1) lib/action_dispatch/middleware/debug_exceptions.rb:32:in `call'
web-console (4.0.2) lib/web_console/middleware.rb:132:in `call_app'
web-console (4.0.2) lib/web_console/middleware.rb:28:in `block in call'
web-console (4.0.2) lib/web_console/middleware.rb:17:in `catch'
web-console (4.0.2) lib/web_console/middleware.rb:17:in `call'
actionpack (6.0.3.1) lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
railties (6.0.3.1) lib/rails/rack/logger.rb:37:in `call_app'
railties (6.0.3.1) lib/rails/rack/logger.rb:26:in `block in call'
activesupport (6.0.3.1) lib/active_support/tagged_logging.rb:80:in `block in tagged'
activesupport (6.0.3.1) lib/active_support/tagged_logging.rb:28:in `tagged'
activesupport (6.0.3.1) lib/active_support/tagged_logging.rb:80:in `tagged'
railties (6.0.3.1) lib/rails/rack/logger.rb:26:in `call'
sprockets-rails (3.2.1) lib/sprockets/rails/quiet_assets.rb:13:in `call'
actionpack (6.0.3.1) lib/action_dispatch/middleware/remote_ip.rb:81:in `call'
actionpack (6.0.3.1) lib/action_dispatch/middleware/request_id.rb:27:in `call'
rack (2.2.2) lib/rack/method_override.rb:24:in `call'
rack (2.2.2) lib/rack/runtime.rb:22:in `call'
activesupport (6.0.3.1) lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
actionpack (6.0.3.1) lib/action_dispatch/middleware/executor.rb:14:in `call'
actionpack (6.0.3.1) lib/action_dispatch/middleware/static.rb:126:in `call'
rack (2.2.2) lib/rack/sendfile.rb:110:in `call'
actionpack (6.0.3.1) lib/action_dispatch/middleware/host_authorization.rb:82:in `call'
rack-mini-profiler (2.0.0) lib/mini_profiler/profiler.rb:312:in `call'
webpacker (5.1.1) lib/webpacker/dev_server_proxy.rb:25:in `perform_request'
rack-proxy (0.6.5) lib/rack/proxy.rb:57:in `call'
railties (6.0.3.1) lib/rails/engine.rb:527:in `call'
puma (4.3.5) lib/puma/configuration.rb:228:in `call'
puma (4.3.5) lib/puma/server.rb:713:in `handle_request'
puma (4.3.5) lib/puma/server.rb:472:in `process_client'
puma (4.3.5) lib/puma/server.rb:328:in `block in run'
puma (4.3.5) lib/puma/thread_pool.rb:134:in `block in spawn_thread'

@SamSaffron
Copy link
Member

There is no clear way of winning this fight, if we move to prepend we break alias method, if we keep alias method we break prepend.

I guess we can make this optional like some other patches.

@glaszig
Copy link
Contributor Author

glaszig commented Jun 23, 2020

alright. i’m gonna integrate with #444.

@glaszig
Copy link
Contributor Author

glaszig commented Jun 24, 2020

restored the old method and split the two into two files. otherwise works the same as the net::http patch. see readme.

@glaszig
Copy link
Contributor Author

glaszig commented Jun 24, 2020

is somebody taking care of the test suite?

@codecov-io
Copy link

codecov-io commented Jan 23, 2021

Codecov Report

Merging #451 (6740420) into master (c774215) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #451   +/-   ##
=======================================
  Coverage   87.69%   87.69%           
=======================================
  Files          18       18           
  Lines        1243     1243           
=======================================
  Hits         1090     1090           
  Misses        153      153           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c774215...6740420. Read the comment docs.

@glaszig
Copy link
Contributor Author

glaszig commented Jan 24, 2021

bump

@OsamaSayegh OsamaSayegh merged commit 396b2a1 into MiniProfiler:master Jan 25, 2021
@OsamaSayegh
Copy link
Collaborator

Thank you @glaszig!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants