]> git.ipfire.org Git - thirdparty/sqlalchemy/alembic.git/commitdiff
Use `-> None` in script templates by default
authorNikita Sobolev <mail@sobolevn.me>
Thu, 21 Apr 2022 16:39:08 +0000 (12:39 -0400)
committersqla-tester <sqla-tester@sqlalchemy.org>
Thu, 21 Apr 2022 16:39:08 +0000 (12:39 -0400)
<!-- Provide a general summary of your proposed changes in the Title field above -->

### Description

When working together with `mypy` we have a problem with the default template.
`mypy` raises errors on all migrations when used with https://mypy.readthedocs.io/en/stable/config_file.html#confval-disallow_untyped_defs

So, there are several options:
1. Remove this flag for `[mypy-db.alembic.*]` in `mypy.ini`. This is not a very good idea, because custom code that might get written in this directory might be not fully typed.
2. Edit our own template. This is what we end up doing right now, but this requires some manual work and many users might miss it
3. Update the source! I think that this is harmless (because only python3.6+ is supported), but allows better type-checking

### Checklist

Openning this as a draft for now. Will fix tests after I can see what is failing in the CI.
Issue is also on its way! ðŸ™‚

<!-- go over following points. check them with an `x` if they do apply, (they turn into clickable checkboxes once the PR is submitted, so no need to do everything at once)

-->

This pull request is:

- [ ] A documentation / typographical error fix
- Good to go, no issue or tests are needed
- [ ] A short code fix
- please include the issue number, and create an issue if none exists, which
  must include a complete example of the issue.  one line code fixes without an
  issue and demonstration will not be accepted.
- Please include: `Fixes: #<issue number>` in the commit message
- please include tests.   one line code fixes without tests will not be accepted.
- [x] A new feature implementation
- please include the issue number, and create an issue if none exists, which must
  include a complete example of how the feature would look.
- Please include: `Fixes: #<issue number>` in the commit message
- please include tests.

Fixes https://github.com/sqlalchemy/alembic/issues/764

**Have a nice day!**

Closes: #1012
Pull-request: https://github.com/sqlalchemy/alembic/pull/1012
Pull-request-sha: c1d17b202f414a97e6215cc2a513509462a9db09

Change-Id: Ib077157b5ebd697bf648644a954ed5a9a3b33080

alembic/templates/async/env.py
alembic/templates/async/script.py.mako
alembic/templates/generic/env.py
alembic/templates/generic/script.py.mako
alembic/templates/multidb/env.py
alembic/templates/multidb/script.py.mako
alembic/templates/pylons/env.py
alembic/templates/pylons/script.py.mako
docs/build/unreleased/764.rst [new file with mode: 0644]
tests/test_script_production.py

index 61030dcf6b229768456da93f15be0e6b03f35b19..c0071b345b652d0ad19937b17475b56191dfb8a0 100644 (file)
@@ -3,6 +3,7 @@ from logging.config import fileConfig
 
 from sqlalchemy import engine_from_config
 from sqlalchemy import pool
+from sqlalchemy.engine import Connection
 from sqlalchemy.ext.asyncio import AsyncEngine
 
 from alembic import context
@@ -28,7 +29,7 @@ target_metadata = None
 # ... etc.
 
 
-def run_migrations_offline():
+def run_migrations_offline() -> None:
     """Run migrations in 'offline' mode.
 
     This configures the context with just a URL
@@ -52,14 +53,14 @@ def run_migrations_offline():
         context.run_migrations()
 
 
-def do_run_migrations(connection):
+def do_run_migrations(connection: Connection) -> None:
     context.configure(connection=connection, target_metadata=target_metadata)
 
     with context.begin_transaction():
         context.run_migrations()
 
 
-async def run_migrations_online():
+async def run_migrations_online() -> None:
     """Run migrations in 'online' mode.
 
     In this scenario we need to create an Engine
index 2c0156303a8df3ffdc9de87765bf801bf6bea4a5..55df2863d206fa1678abb4c92e90c45d3f85c114 100644 (file)
@@ -16,9 +16,9 @@ branch_labels = ${repr(branch_labels)}
 depends_on = ${repr(depends_on)}
 
 
-def upgrade():
+def upgrade() -> None:
     ${upgrades if upgrades else "pass"}
 
 
-def downgrade():
+def downgrade() -> None:
     ${downgrades if downgrades else "pass"}
index edeff1d5f4f97f71b639fadbb298467a273d2203..6626bfd0c4d6bc29d144218599d446fbbfb238e7 100644 (file)
@@ -26,7 +26,7 @@ target_metadata = None
 # ... etc.
 
 
-def run_migrations_offline():
+def run_migrations_offline() -> None:
     """Run migrations in 'offline' mode.
 
     This configures the context with just a URL
@@ -50,7 +50,7 @@ def run_migrations_offline():
         context.run_migrations()
 
 
-def run_migrations_online():
+def run_migrations_online() -> None:
     """Run migrations in 'online' mode.
 
     In this scenario we need to create an Engine
index 2c0156303a8df3ffdc9de87765bf801bf6bea4a5..55df2863d206fa1678abb4c92e90c45d3f85c114 100644 (file)
@@ -16,9 +16,9 @@ branch_labels = ${repr(branch_labels)}
 depends_on = ${repr(depends_on)}
 
 
-def upgrade():
+def upgrade() -> None:
     ${upgrades if upgrades else "pass"}
 
 
-def downgrade():
+def downgrade() -> None:
     ${downgrades if downgrades else "pass"}
index 687f584313a8cd7d29e44ee413b007ec92113fdd..f787824c6a4111d321b61f51fe37a819e2d1451c 100644 (file)
@@ -43,7 +43,7 @@ target_metadata = {}
 # ... etc.
 
 
-def run_migrations_offline():
+def run_migrations_offline() -> None:
     """Run migrations in 'offline' mode.
 
     This configures the context with just a URL
@@ -79,7 +79,7 @@ def run_migrations_offline():
                 context.run_migrations(engine_name=name)
 
 
-def run_migrations_online():
+def run_migrations_online() -> None:
     """Run migrations in 'online' mode.
 
     In this scenario we need to create an Engine
index c3970a521906b30ed9ed9e8662d73f8ee278ce08..946ab6a9740355de8808a18ad1df37a17bc929e3 100644 (file)
@@ -19,11 +19,11 @@ branch_labels = ${repr(branch_labels)}
 depends_on = ${repr(depends_on)}
 
 
-def upgrade(engine_name):
+def upgrade(engine_name: str) -> None:
     globals()["upgrade_%s" % engine_name]()
 
 
-def downgrade(engine_name):
+def downgrade(engine_name: str) -> None:
     globals()["downgrade_%s" % engine_name]()
 
 <%
@@ -35,11 +35,11 @@ def downgrade(engine_name):
 
 % for db_name in re.split(r',\s*', db_names):
 
-def upgrade_${db_name}():
+def upgrade_${db_name}() -> None:
     ${context.get("%s_upgrades" % db_name, "pass")}
 
 
-def downgrade_${db_name}():
+def downgrade_${db_name}() -> None:
     ${context.get("%s_downgrades" % db_name, "pass")}
 
 % endfor
index 18d47c8547cf9c30e5dfbf034e60813353bddeee..99bc19ef12a606638c8764548436073248b22e6a 100644 (file)
@@ -38,7 +38,7 @@ meta = __import__(
 target_metadata = None
 
 
-def run_migrations_offline():
+def run_migrations_offline() -> None:
     """Run migrations in 'offline' mode.
 
     This configures the context with just a URL
@@ -60,7 +60,7 @@ def run_migrations_offline():
         context.run_migrations()
 
 
-def run_migrations_online():
+def run_migrations_online() -> None:
     """Run migrations in 'online' mode.
 
     In this scenario we need to create an Engine
index 2c0156303a8df3ffdc9de87765bf801bf6bea4a5..55df2863d206fa1678abb4c92e90c45d3f85c114 100644 (file)
@@ -16,9 +16,9 @@ branch_labels = ${repr(branch_labels)}
 depends_on = ${repr(depends_on)}
 
 
-def upgrade():
+def upgrade() -> None:
     ${upgrades if upgrades else "pass"}
 
 
-def downgrade():
+def downgrade() -> None:
     ${downgrades if downgrades else "pass"}
diff --git a/docs/build/unreleased/764.rst b/docs/build/unreleased/764.rst
new file mode 100644 (file)
index 0000000..184c0ac
--- /dev/null
@@ -0,0 +1,6 @@
+.. change::
+    :tags: feature, typing
+    :tickets: 764
+
+    Adds typing annotations to ``env.py`` and migration templates.
+    Pull request by Nikita Sobolev.
index 2edae26779fdf4ed9b5e22c9bb5c323982bf9843..05167f0b4865107da7d7f72c67cca97179fe011b 100644 (file)
@@ -618,7 +618,7 @@ def downgrade():
         assert (
             (
                 """
-def upgrade():
+def upgrade() -> None:
     # ### commands auto generated by Alembic - please adjust! ###
     op.create_table('test_table',
     sa.Column('id', sa.Integer(), nullable=False),