Skip to content

feat: Set table name based on flag or environment variable#932

Merged
mfridman merged 3 commits intopressly:mainfrom
d6o:custom-table-env-var
Apr 3, 2025
Merged

feat: Set table name based on flag or environment variable#932
mfridman merged 3 commits intopressly:mainfrom
d6o:custom-table-env-var

Conversation

@d6o
Copy link
Copy Markdown
Contributor

@d6o d6o commented Apr 2, 2025

Created a new environment variable, GOOSE_TABLE, that enables users to configure a custom table name. Updated the SetTableName logic to prioritize flags over the new environment variable, following the previous behaviour for other env vars. Implemented a helper function, firstNonEmpty, to facilitate flexible value selection. Wrote unit tests to validate the function. Updated the README.md with the newly create GOOSE_TABLE environment variable.

Example:

export GOOSE_TABLE=custom.table_name
goose up

Please let me know if any other change is required.

@d6o d6o changed the title Set table name based on flag or environment variable feat: Set table name based on flag or environment variable Apr 2, 2025
@mfridman
Copy link
Copy Markdown
Collaborator

mfridman commented Apr 3, 2025

I think this seems reasonable, will take a pass in the next few days.

goose.SetTableName(*table)

// Envvars should have lower priority than flags.
goose.SetTableName(firstNonEmpty(*table, envConfig.table))
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I don't think this will work, since *table will be set to the default from line 31:

flags.String("table", "goose_db_version", "migrations table name")

I think what might work here is:

- goose.SetTableName(firstNonEmpty(*table, envConfig.table))
+ goose.SetTableName(firstNonEmpty(*table, envConfig.table, "goose_db_version"))

And then remove the flag default:

- table        = flags.String("table", "goose_db_version", "migrations table name")
+ table        = flags.String("table", "", "migrations table name")

I believe that should maintain the order of precedence of flag > env > default.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks for looking into this @mfridman! I’ve made a change that should fix it. Please let me know if there are any other changes.

d6o added 2 commits April 3, 2025 08:52
Replaced the hardcoded DefaultTable variable with the goose package's DefaultTablename. This ensures a single source of truth for the default table name and improves maintainability.
@mfridman
Copy link
Copy Markdown
Collaborator

mfridman commented Apr 3, 2025

Thank you for the contribution, lgtm.

@mfridman mfridman merged commit 46ae662 into pressly:main Apr 3, 2025
4 checks passed
@d6o d6o deleted the custom-table-env-var branch April 3, 2025 17:55
@d6o
Copy link
Copy Markdown
Contributor Author

d6o commented Apr 3, 2025

Thanks @mfridman! Do you have any estimates of when a new release might be created?

@mfridman
Copy link
Copy Markdown
Collaborator

mfridman commented Apr 7, 2025

Within the next few weeks, I typically try and stagger the releases to avoid too many updates on consumers.

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