-
Notifications
You must be signed in to change notification settings - Fork 375
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
Implement ActiveRecord integration configuration #451
Implement ActiveRecord integration configuration #451
Conversation
887e679
to
afe57f3
Compare
01dc89d
to
879b111
Compare
afe57f3
to
c412d1c
Compare
879b111
to
c94bfb7
Compare
c94bfb7
to
c9873f8
Compare
@@ -0,0 +1,301 @@ | |||
require 'uri' | |||
|
|||
# NOTE: This code is copied directly from ActiveRecord. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WDYT about putting this file in a folder named vendor
? Similarily how gems are put in ROOT/vendor
but here it would be something like:
lib/ddtrace/contrib/active_record/vendor/configuration/connection_specification.rb
Its a small and cosmetic change, but it would make me feel much better about including files from different projects verbatim.
We could actually insert this file without any modifications too, and add small note in the vendor folder, and exclude the whole prefix in the Rubocop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm, I like that idea. Would make the Rubocop and other stuff much simpler to deal with too. I'll try this.
cc96863
to
91e19ad
Compare
Following from #450 , this pull request implements the integration configuration core for ActiveRecord, and leverages its multiplexing feature to implement the API for multi-database configuration (based on #404), permitting multiple databases to be configured like so: