Open trilader opened 2 years ago
Thanks for the report.
It looks like in this function: https://github.com/nickjj/flask-db/blob/109199f8c4a01e923afd1fe6fac34624e48c4544/flask_db/cli.py#L120-L129
If we replaced return None
with sys.exit(subprocess.call(cmdline))
it should properly return the exit code. Would you mind giving that a test run? You'll also need to import sys
to make that work.
I just tested your suggestion and it works as expected. Do you want a pull request for this?
A PR would be much appreciated but I think if we do this we should probably adjust all commands to throw a proper exit code. Would you mind testing them to see if it works without any changes since they use a different method?
The seed
command is at:
We might need to wrap that in a sys.exit()
too.
There's also the reset
command too:
https://github.com/nickjj/flask-db/blob/109199f8c4a01e923afd1fe6fac34624e48c4544/flask_db/cli.py#L86
I'm not sure if this carries over the exit code too.
For the reset
command it definitely looks like a good idea. The docs say invoke
returns a result object which includes the exit_code
so we can sys.exit
with that. In my simple case it does what I expect.
Regarding the seed
command: As far as I know exec
doesn't return anything by itself but it just executes the string or file passed to it. If that has a sys.exit
in it, it will exit Flask-DB
with whatever it gets passed as well. But we can't know what the user will put into their seed file so we can't rely on that.
Hmm for the seed
command what if we wrapped the exec()
call in a try / except and on any error then we sys.exit(1)
?
Any updates on this? Ran into the same issue when trying to set up running migrations as part of an automated Azure DevOps pipeline.
I had an error in a migration and I noticed that Flask-DB discards the exit code from alembic.
You can test this easily by putting, e.g.,
raise ValueError("This should fail")
into a migrationsupgrade
method. When runningalembic upgrade head
directly the process exits with 1. When runningflask db migrate upgrade head
the exception gets printed but the commands exit code is set to 0. This is an issue when running database migrations with a configuration management system, in my case Ansible, as the error will silently be ignored.