-
Notifications
You must be signed in to change notification settings - Fork 545
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
Timeout Flag #576
Comments
Yep, sounds like a great idea since we can propagate a ctx down. We can also take this a bit further (although probably captured in a separate issue/PR) by propagating a cancellation by leveraging https://pkg.go.dev/os/signal#NotifyContext in the CLI. |
Not sure if you were looking to contribute, but I can go ahead and implement a timeout and propagate a cancellation. |
@mfridman I can whip up a PR, let me know if you would like me to. |
Sure thing, PRs always appreciated. |
One of the features I personally feel that the goose cli is missing is a
--timeout
flag for migrations.This would require migrations finish within an allotted amount of time or else rollback back the migration.
I took a precursory look at the code base and with the new context aware functions this might be pretty straight forward to implement by putting a timeout on the context if
--timeout
is specified, and then passing that intosql.DB.BeginTx
.I think migrations with multiple transactions may cause a bit more complexity but still doable.
Thoughts?
The text was updated successfully, but these errors were encountered: