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

Suppressing logging of internal grate migrations doesn't appear to be working correctly #519

Closed
JaDuyve opened this issue May 12, 2024 · 3 comments

Comments

@JaDuyve
Copy link
Contributor

JaDuyve commented May 12, 2024

Describe the bug
Grate still logs everything while running grate internal migrations.

From debugging the code I think the reason it still logs everything when running internal migrations is because the logger is created before setting the new configuration into the DbMigrator instance. The method IsInternalMigration uses the configuration from inside the DbMigrator instance as you can see inside code snippet 2.

internal IDbMigrator DbMigrator { get; private init; }
private string LogCategory => $"Grate.Migration{(IsInternalMigration() ? ".Internal" : "")}";
public GrateConfiguration Configuration
{
get => DbMigrator.Configuration;
private init
{
_logger = _loggerFactory.CreateLogger(LogCategory);
DbMigrator = (IDbMigrator) DbMigrator.Clone();
DbMigrator.Configuration = value;
DbMigrator.Logger = _logger;
}
}

private bool IsInternalMigration() => GrateEnvironment.Internal == this.Configuration?.Environment
|| GrateEnvironment.InternalBootstrap == this.Configuration?.Environment;

I tried lowering the line where the logger is created below the line where the new configuration (value) is set into the newly created DbMigrator instance, and it worked from what I can see. Although still some things are logged because they are logged with a logger provided by dependency injection and not the custom created logger (I'm not sure if this is a bad thing).

This is what I get when I apply the above-mentioned change.

Initializing connections.
Running grate v1.0.0 (build date 05/12/2024 17:58:26) against localhost,1435 - TestDb.
Looking in D:\Repos\TestDb for scripts to run.
================================================================================
Setup, Backup, Create/Restore/Drop
================================================================================
================================================================================
Grate Structure
================================================================================
Initializing connections.
 Versioning TestDb database with version 1.0.0.
Initializing connections.
 Versioning TestDb database with version 1.0.0.
Initializing connections.
 Versioning TestDb database with version 1.0.0.
Initializing connections.
 Versioning TestDb database with version 1.0.0.
================================================================================
Versioning
================================================================================
 Migrating TestDb from version 0.0.0.0 to 0.0.0.1.
 Versioning TestDb database with version 0.0.0.1.
================================================================================
Migration Scripts
================================================================================
Skipping 'BeforeMigration', beforeMigration does not exist.
Skipping 'AlterDatabase', alterDatabase does not exist.
Skipping 'Run After Create Database', runAfterCreateDatabase does not exist.
Skipping 'Run Before Update', runBeforeUp does not exist.

Looking for Update scripts in "D:\Repos\TestDb\./ddl/05.up.OneTime". These should be one time only scripts.
--------------------------------------------------------------------------------
  Running '20240312_01_CreateTables.sql'.
--------------------------------------------------------------------------------

Skipping 'Run First After Update', runFirstAfterUp does not exist.
Skipping 'Functions', functions does not exist.
Skipping 'Views', views does not exist.
Skipping 'Stored Procedures', sprocs does not exist.
Skipping 'Triggers', triggers does not exist.
Skipping 'Indexes', indexes does not exist.
Skipping 'Run after Other Anytime Scripts', runAfterOtherAnyTimeScripts does not exist.
Skipping 'Permissions', permissions does not exist.
Skipping 'AfterMigration', afterMigration does not exist.


grate v1.0.0 (build date 05/12/2024 17:58:26) has grated your database (TestDb)! You are now at version 0.0.0.1. All changes and backups can be found at "C:\Users\jaduyve\AppData\Local\grate\migrations\TestDb\localhost_1435\2024-05-12T17_58_29.6460636_02_00".

To Reproduce
Just run grate when the internal migrations haven't been run.

Expected behavior
Logs with log level below warning during internal migration should not be visible.

Desktop (please complete the following information):

  • OS: Windows, MacOS
  • Version: 1.7.3
  • Database: MSSQL
@JaDuyve
Copy link
Contributor Author

JaDuyve commented May 12, 2024

I created a PR which applies the above-mentioned change. Do you think this change would be suitable for this issue 😃? #520

@erikbra
Copy link
Owner

erikbra commented May 15, 2024

Thank you. Bad testing on my part there 😄 unit tests doesn't test everything...

@erikbra
Copy link
Owner

erikbra commented May 15, 2024

Fixed in #520

@erikbra erikbra closed this as completed May 15, 2024
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

No branches or pull requests

2 participants