From: Mike Bayer <mike_mp@zzzcomputing.com>
Date: Tue, 17 Sep 2019 21:55:44 +0000 (-0400)
Subject: Render a single "pass" only for UpgradeOps, DowngradeOps
X-Git-Tag: rel_1_2_0~13^2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=10795bfff96bcc0efca2212b92846b36fd25b786;p=thirdparty%2Fsqlalchemy%2Falembic.git

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 24eeb4e7..4fe66eca 100644
--- 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
index 00000000..740f11ce
--- /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 e57585af..05d01335 100644
--- 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):