Return exit code from alembic

[trilader]

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 migrations upgrade method. When running alembic upgrade head directly the process exits with 1. When running flask 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.

[nickjj]

Thanks for the report.

It looks like in this function:

flask-db/flask_db/cli.py

Lines 120 to 129 in 109199f

	def migrate(alembic_args):
	"""Wrap the alembic CLI tool (defaults to running upgrade head)."""
	if not alembic_args:
	alembic_args = ("upgrade", "head")
	cmdline = ["alembic"] + list(alembic_args)
	subprocess.call(cmdline)
	return None

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.

[trilader]

I just tested your suggestion and it works as expected. Do you want a pull request for this?

[nickjj]

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:

flask-db/flask_db/cli.py

Line 101 in 109199f

exec(open(seeds_path).read())

We might need to wrap that in a sys.exit() too.

There's also the reset command too:

flask-db/flask_db/cli.py

Line 86 in 109199f

ctx.invoke(seed)

I'm not sure if this carries over the exit code too.

[trilader]

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.

[nickjj]

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)?

[mzhukovs]

Any updates on this? Ran into the same issue when trying to set up running migrations as part of an automated Azure DevOps pipeline.