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 Logger to Output Query Logs #4

Merged
merged 7 commits into from
Jul 4, 2024
Merged

Conversation

meri025
Copy link
Contributor

@meri025 meri025 commented Jul 4, 2024

Summary

This pull request introduces a logger to output query logs when the logger is specified in the setup. This enhancement will allow for better tracking and debugging of queries, especially during development and testing.

Changes

  • Added a logger that can be configured to output query logs.
  • Modified the setup to accept a logger configuration.
  • Updated README.md to reflect these changes.
  • The logger configuration is optional. If not specified, the application will continue to run without logs.

Other Information

  • Corrected a typo in the README.md from "DatabaseRewinder" to the correct term.

Checklist

Copy link
Contributor

@genya0407 genya0407 left a comment

Choose a reason for hiding this comment

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

Thank you for contributing!
I have some comments. Please check them.

README.md Outdated Show resolved Hide resolved
@@ -89,7 +89,7 @@ MysqlRewinder.setup(db_configs, adapter: :mysql2)
If you want to use MysqlRewinder with ActiveRecord, do the following:

* Generate db_configs from `ActiveRecord::Base.configurations`
* Pass `ActiveRecord::SchemaMigration.new(nil).table_name` and `ActiveRecord::Base.internal_metadata_table_name` to `DatabaseRewinder.setup` as `except_tables`
* Pass `ActiveRecord::SchemaMigration.new(nil).table_name` and `ActiveRecord::Base.internal_metadata_table_name` to `MysqlRewinder.setup` as `except_tables`
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks!

lib/mysql_rewinder/cleaner.rb Outdated Show resolved Hide resolved
lib/mysql_rewinder/cleaner.rb Outdated Show resolved Hide resolved
private

def log_sql(sql)
return yield unless @logger.debug?
Copy link
Contributor

Choose a reason for hiding this comment

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

With this, I understand that the logging code below is executed even if @logger is NullLogger, that isn't desirable.

Instead, we should set @logger = nil if logger is not passed.
Then we can write like:

def log_and_execute(sql)
  return @client.execute(sql) unless @logger&.debug?

  ...
end

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Fixed for 69070ff

meri025 and others added 6 commits July 4, 2024 17:48
Co-authored-by: Yusuke Sangenya <[email protected]>
The type definition of the logger faced the same issue as described in ruby/rbs#1482, so it was set to untyped.
@genya0407 genya0407 merged commit 93e520d into DeNA:trunk Jul 4, 2024
6 checks passed
@meri025 meri025 deleted the feature/add_logger branch July 4, 2024 09:51
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.

2 participants