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

Grate internal scripts fail when using CreateDatabase scripts #512

Closed
JaDuyve opened this issue May 6, 2024 · 4 comments · Fixed by #514
Closed

Grate internal scripts fail when using CreateDatabase scripts #512

JaDuyve opened this issue May 6, 2024 · 4 comments · Fixed by #514
Labels
bug Something isn't working

Comments

@JaDuyve
Copy link
Contributor

JaDuyve commented May 6, 2024

Describe the bug
The grate internal structure scripts fail when using CreateDatabase scripts. From what I can see, it's caused because the DbMigrator instance is reused from the CreateDatabase scripts. This instance already has the private instance of _tokens initialized. Because it's already initialized, the custom UserTokens for the grate internal scripts will not be added to _tokens causing TokenReplacer not replacing all supposed tokens and intern, the script will fail.

These tokens aren't taken into account.

private async Task<GrateConfiguration> GetBootstrapInternalGrateConfiguration(string internalFolderName) =>
await GetInternalGrateConfiguration(internalFolderName, "grate-internal") with
{
UserTokens = [
"ScriptsRunTable=GrateScriptsRun",
"ScriptsRunErrorsTable=GrateScriptsRunErrors",
"VersionTable=GrateVersion"
],
DeferWritingToRunTables = true,
Environment = GrateEnvironment.InternalBootstrap,
Baseline = false
};

Because _tokens is already initialized.

private Dictionary<string, string?>? _tokens;
private string ReplaceTokensIn(string sql)
{
_tokens ??= new TokenProvider(Configuration, Database).GetTokens();
return TokenReplacer.ReplaceTokens(_tokens, sql);
}

I think this is issue will occur when anything has run before grate structure.

Could the solution be to create for the grate internal scripts a new instance of GrateMigrator?

To Reproduce
Run grate for new database with CreateDatabase scripts.

Expected behavior
Grate internal structure scripts run without errors.

Screenshots

Initializing connections.
Running grate v1.0.0 (build date 05/06/2024 23:17:39) against localhost,1435 - TestDatabase.
Looking in D:\Repos\TestDatabase for scripts to run.
================================================================================
Setup, Backup, Create/Restore/Drop
================================================================================
  Running '20240311_01_Config.sql'.
  Running '20240311_02_SetConfig.ENV.LOCAL.sql'.
  Running '20240311_03_CreateLogin.sql'.
  Running '20240311_04_CreateDatabase.sql'.
  Running '20240311_05_Wait.sql'.
================================================================================
Grate Structure
================================================================================
Initializing connections.
Running grate v1.0.0 (build date 05/06/2024 23:17:39) against localhost,1435 - TestDatabase.
Looking in C:\Users\jaduyve\AppData\Local\Temp\tmpd4ubxq.tmp for scripts to run.
================================================================================
Setup, Backup, Create/Restore/Drop
================================================================================
================================================================================
Grate Structure
================================================================================
================================================================================
Versioning
================================================================================
 Migrating TestDatabase from version 0.0.0.0 to 1.0.0.
 Versioning TestDatabase database with version 1.0.0.
================================================================================
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 "C:\Users\jaduyve\AppData\Local\Temp\tmpd4ubxq.tmp\up". These should be one time only scripts.
--------------------------------------------------------------------------------
  Running 'grate-internal/01_create_schema_grate.ENV.GrateInternalBoostrap-a61456d0-e00a-4933-b692-c6a5d7d51539.sql'.
  Running 'grate-internal/02_create_scripts_run_table.sql'.
grate-internal/02_create_scripts_run_table.sql: Incorrect syntax near '{'.
Skipping 'Permissions', permissions does not exist.
Skipping 'AfterMigration', afterMigration does not exist.
An error occurred: Invalid object name 'grate.GrateScriptsRun'.

Desktop (please complete the following information):

  • OS: Windows/Macos
  • Version: 1.7.0
  • Database: MSSQL
@erikbra
Copy link
Owner

erikbra commented May 7, 2024

Aha! Good catch! I thought I had tests covering this, but apparently not (strange...). I'll try to write some, and fix the issue.

@erikbra erikbra added the bug Something isn't working label May 7, 2024
erikbra added a commit that referenced this issue May 7, 2024
* Make sure the replacement tokens are reset when running the internal migrations
@erikbra
Copy link
Owner

erikbra commented May 7, 2024

I think PR #514 should solve this, do you agree? I make sure to set the _tokens to null whenever I clone the DbMigrator

erikbra added a commit that referenced this issue May 7, 2024
…ts (#514)

* Make sure the replacement tokens are reset when running the internal migrations
@JaDuyve
Copy link
Contributor Author

JaDuyve commented May 8, 2024

@erikbra This fixes the issue I'm having. Thanks a lot!

@erikbra
Copy link
Owner

erikbra commented May 8, 2024

Lovely, @JaDuyve! It is included in release 1.7.2. Thanks for a detailed bug report and solid analysis 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants