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

Check that @config exists. Fixes #413 #528

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

calh
Copy link

@calh calh commented Jun 26, 2019

This fixes #413. It depends on PR #526 only just for running the tests. The real fix is just a one-liner to check that @config exists in the database adapter. If it doesn't exist, the only side effect is that the message [Shard: name] isn't prepended to the query log.

It appears that database adapters like ibm_db and sqlserver do not use an internal config variable called @config, and the consequence is that the rails startup breaks. This quick patch at least gets people up and running.

I'm working on a more elegant fix in https://github.com/calh/octopus/tree/ibm_db, but it's not going very well. The ibm_db gem breaks on rails 5.2. I think the only two options around this would be to pull Rails 5.2 support in octopus, or try to pursue a fix in the ibm_db gem. I'd appreciate any advice on this!

calh added 4 commits June 21, 2019 15:38
Running cucumber gives the error:

cannot load such file -- test/unit (LoadError)
These weren't supposed to make it into this PR
Error message is:

ActiveRecord::StatementInvalid: NoMethodError: undefined method `[]' for
nil:NilClass:

  from octopus/lib/octopus/abstract_adapter.rb:23:in `octopus_shard'
  from octopus/lib/octopus/abstract_adapter.rb:12:in `instrument'
  from vendor/bundle/ruby/2.3.0/gems/activerecord-4.2.11.1/lib/active_record/connection_adapters/abstract_adapter.rb:478:in `log'

This has shown to surface in the ibm_db and sqlserver adapters.
@calh
Copy link
Author

calh commented Jun 27, 2019

ibmdb/ruby-ibmdb#88

Trying this from another angle too, hopefully IBM will accept my patch over there.

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