Unusable display of alembic_check output

[lachaib]

Describe the use case
We're using alembic check in our CI to decorate pull requests with potentially missing DB changes.
The output of alembic check is added as a comment in the pull request, but it's hardly readable.

Example Use

New upgrade operations detected: [('remove_table', Table('some_table', MetaData(), Column('id', UUID(), table=<some_table>, primary_key=True, nullable=False, default=CallableColumnDefault(<function uuid4 at 0x7f18789fafc0>)), Column('url', String(), table=<some_table>, nullable=False), Column('other_table_id', String(), ForeignKey('other_table.id'), table=<some_table>, nullable=False), schema=None)), ('remove_table', Table('another_table', MetaData(), Column('id', UUID(), table=<another_table>, primary_key=True, nullable=False, default=CallableColumnDefault(<function uuid4 at 0x7f187883f4c0>)), Column('url', String(), table=<another_table>, nullable=False), Column('a_column', String(), table=<another_table>), Column('b_column', String(), table=<another_table>), Column('external_id', Integer(), ForeignKey('external_table.id'), table=<another_table>, nullable=False), schema=None))]

What could be output instead, which would summarize it better:

op.remove_table('some_table', if_exists=True)
op.remove_table('another_table', if_exists=True)

or even

DROP TABLE IF EXISTS some_table;
DROP TABLE IF EXISTS another_table;

Additional context

Note that in above expectations I'd be delighted that we can have the if_exists option which is currently added by my rewriter.

What I can propose as solutions:

• add options as_sql or as_ops to check command, so that we can specify output formats
• add least rework the AutogenerateDiffsDetected so that I may make a wrapper above the command and catch it to make the display that suits me best. passing diffs/migration_script would be sufficient to change rendering
• both of the above for people who want to make an even different display format

Have a nice day!

[zzzeek]

I am onboard with replacing the use of repr(), which dumps out a lot of extranous detail, with something that is beautified and succinct to read.

Where it goes into duplicating what alembic revision --autogenerate does (e.g. dumping out working Python code) or then even jumping ahead into essentially alembic revision --autogenerate; alembic upgrade head --sql I'm pretty uncomfortable with.

is it your goal to cut and paste working code from the output? I'd rather add features to --autogenerate, I think there are already tickets to provide options for autogenerate to dump python code to the console instead. check is supposed to give a yes or no answer, that's all it's intended to do. I'd sooner remove the listing of changes rather than make check responsible for formatting them into fully correct Python or SQL code.

[zzzeek]

as far as AutogenerateDiffsDetected, absolutely we should rework it so that it accepts the revision structure directly, then you can catch it and do what you want, sure.

[lachaib]

Well, we're using check especially because it's giving the yes/no answer and we can use the status code to fail a pipeline.

I'll start making a small PR to make the AutogenerateDiffDetected contain contextual info, and we can discuss and iterate over it if you'd like

[zzzeek]

that's what check is for. additional output should give readers a clue where to look in their applications but it shouldn't pretend to be workable code; once we start dumping out code, now that code has to be correct, then it has to work in situation X, Y, then it needs options like batch rendering, it becomes a complete reinvention of the revision --autogenerate command. you get the idea

[lachaib]

I totally understand your position, and I've submitted the minimal change for me to do whatever I feel will be useful with it without alembic having to make plenty of assumptions.
However, prior to existence of alembic check, our situation was that we used revision --autogenerate, had to count the number of pass statements in the generated script, plus dealing with the file that was hanging in the workspace.
alembic check is convenient for that matter.

Having the context would help me compute clues about the situation ("you would generate a drop table, but I could find a table definition matching here, maybe a missing import"), but here I just get a repr that can gives me nothing workable

[zzzeek]

fixed in 3d33cfa