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

Add component to comment with the database role #104

Closed
wants to merge 1 commit into from

Conversation

geoffharcourt
Copy link
Contributor

@geoffharcourt geoffharcourt commented May 23, 2020

With multiple databases in Rails 6, it can be difficult to tell from
logging if your database query is being made against the leader or
follower database. This change adds a new component, db_role, which
can be used with the database host and name to annotate the query with
the current ActiveRecord role.

Rubies prior to 2.5 are now EOLed and Rails 6 can't be installed on Ruby 2.4,
so I've swapped out Rubies 2.2-2.4 for 2.5 and 2.6 in the build matrix.

Close #102

This change requires #105 to fix an unrelated failing test case.

@geoffharcourt geoffharcourt marked this pull request as draft May 23, 2020 18:07
@geoffharcourt geoffharcourt force-pushed the db-role branch 2 times, most recently from 1d59fe3 to f2efa0b Compare May 23, 2020 18:24
@geoffharcourt geoffharcourt marked this pull request as ready for review May 25, 2020 00:15
@geoffharcourt geoffharcourt force-pushed the db-role branch 2 times, most recently from 5ddc50c to 3ec6a96 Compare March 27, 2021 20:06
@@ -3,6 +3,6 @@ source "https://rubygems.org"
gem "mysql2", "~> 0.4.10"
gem "pg", "~> 0.15"
gem "sqlite3", "~> 1.3.6"
gem "rails", "= 5.2.2.1"
gem "rails", "= 5.2.5"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed due to the mimemagic yank. It was easier to bump this here.

@kyleboe
Copy link

kyleboe commented Oct 12, 2021

Any plans to resolve conflicts & merge this?

@geoffharcourt geoffharcourt force-pushed the db-role branch 3 times, most recently from 86ee543 to 5e2acd7 Compare October 12, 2021 20:03
With multiple databases in Rails 6, it can be difficult to tell from
logging if your database query is being made against the leader or
follower database. This change adds a new component, `db_role`, which
can be used with the database host and name to annotate the query with
the current ActiveRecord role.
@geoffharcourt
Copy link
Contributor Author

I rebased and tried to resolve conflicts since the PR was first opened, looks like there's some things that have drifted in the interim.

@geoffharcourt
Copy link
Contributor Author

Now that Marginalia's functionality has been made part of Rails 7, it doesn't seem likely that this PR is going to get merged. I'm going to close this out rather than plow more time into keeping it up to date.

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.

Allow to report the database reference
2 participants