Render a single "pass" only for UpgradeOps, DowngradeOps

Improved the Python rendering of a series of migration operations such that
a single "pass" is rendered for a :class:`.UpgradeOps` or
:class:`.DowngradeOps` based on if no lines of Python code actually
rendered under the operation, rather than whether or not sub-directives
exist. Removed extra "pass" lines that would generate from the
:class:`.ModifyTableOps` directive so that these aren't duplicated under
operation rewriting scenarios.

Change-Id: Ia2547811565ac24ee0f56ce470f96ec567912de3
Fixes: #550

diff --git a/alembic/autogenerate/render.py b/alembic/autogenerate/render.py
index 24eeb4e7ac431040d58d4cf4d1bf7340bdb476b6..4fe66eca94054fcb5bb289f41215f14aab2ab735 100644 (file)
--- a/alembic/autogenerate/render.py
+++ b/alembic/autogenerate/render.py
@@ -66,14 +66,16 @@ def _render_cmd_body(op_container, autogen_context):
         "# ### commands auto generated by Alembic - please adjust! ###"
     )
 
-    if not op_container.ops:
-        printer.writeline("pass")
-    else:
-        for op in op_container.ops:
-            lines = render_op(autogen_context, op)
+    has_lines = False
+    for op in op_container.ops:
+        lines = render_op(autogen_context, op)
+        has_lines = has_lines or lines
 
-            for line in lines:
-                printer.writeline(line)
+        for line in lines:
+            printer.writeline(line)
+
+    if not has_lines:
+        printer.writeline("pass")
 
     printer.writeline("# ### end Alembic commands ###")
 
@@ -114,7 +116,7 @@ def _render_modify_table(autogen_context, op):
 
         return lines
     else:
-        return ["pass"]
+        return []
 
 
 @renderers.dispatch_for(ops.CreateTableCommentOp)

diff --git a/docs/build/unreleased/550.rst b/docs/build/unreleased/550.rst
new file mode 100644 (file)
index 0000000..740f11c
--- /dev/null
+++ b/docs/build/unreleased/550.rst
@@ -0,0 +1,12 @@
+.. change::
+    :tags: bug, autogenerate
+    :tickets: 550
+
+    Improved the Python rendering of a series of migration operations such that
+    a single "pass" is rendered for a :class:`.UpgradeOps` or
+    :class:`.DowngradeOps` based on if no lines of Python code actually
+    rendered under the operation, rather than whether or not sub-directives
+    exist. Removed extra "pass" lines that would generate from the
+    :class:`.ModifyTableOps` directive so that these aren't duplicated under
+    operation rewriting scenarios.
+

diff --git a/tests/test_script_production.py b/tests/test_script_production.py
index e57585afaf6c2b4bcc5d75321e05dbbe80bfaf33..05d01335432e63578b6092ef0cb392e0215379b2 100644 (file)
--- a/tests/test_script_production.py
+++ b/tests/test_script_production.py
@@ -921,6 +921,61 @@ class RewriterTest(TestBase):
             "    # ### end Alembic commands ###",
         )
 
+    def test_no_needless_pass(self):
+        writer1 = autogenerate.Rewriter()
+
+        @writer1.rewrites(ops.AlterColumnOp)
+        def rewrite_alter_column(context, revision, op):
+            return []
+
+        directives = [
+            ops.MigrationScript(
+                util.rev_id(),
+                ops.UpgradeOps(
+                    ops=[
+                        ops.ModifyTableOps(
+                            "t1",
+                            ops=[
+                                ops.AlterColumnOp(
+                                    "foo",
+                                    "bar",
+                                    modify_nullable=False,
+                                    existing_type=sa.Integer(),
+                                ),
+                                ops.AlterColumnOp(
+                                    "foo",
+                                    "bar",
+                                    modify_nullable=False,
+                                    existing_type=sa.Integer(),
+                                ),
+                            ],
+                        ),
+                        ops.ModifyTableOps(
+                            "t1",
+                            ops=[
+                                ops.AlterColumnOp(
+                                    "foo",
+                                    "bar",
+                                    modify_nullable=False,
+                                    existing_type=sa.Integer(),
+                                )
+                            ],
+                        ),
+                    ]
+                ),
+                ops.DowngradeOps(ops=[]),
+            )
+        ]
+        ctx, rev = mock.Mock(), mock.Mock()
+        writer1(ctx, rev, directives)
+
+        eq_(
+            autogenerate.render_python_code(directives[0].upgrade_ops),
+            "# ### commands auto generated by Alembic - please adjust! ###\n"
+            "    pass\n"
+            "    # ### end Alembic commands ###",
+        )
+
 
 class MultiDirRevisionCommandTest(TestBase):
     def setUp(self):