Skip to content

Commit

Permalink
Fix alembic.ini templates to match configparser file format.
Browse files Browse the repository at this point in the history
In the `alembic.ini` templates, I moved the inline comment about `version_path_separator` to their own lines as required by `configparser`.

### Description

In a recent project, I included the following configuration values in my `alembic.ini`. Note that the last line is the default line from the current generic `alembic.ini` template.

```
# version location specification; This defaults
# to migrations/versions.  When using multiple version
# directories, initial revisions must be specified with --version-path.
# The path separator used here should be the separator specified by "version_path_separator" below.
version_locations = %(here)s/migrations/versions

# version path separator; As mentioned above, this is the character used to split
# version_locations. The default within new alembic.ini files is "os", which uses os.pathsep.
# If this key is omitted entirely, it falls back to the legacy behavior of splitting on spaces and/or commas.
# Valid values for version_path_separator are:
#
# version_path_separator = :
# version_path_separator = ;
# version_path_separator = space
version_path_separator = os  # Use os.pathsep. Default configuration used for new projects.
```

When running `alembic check`, I encountered:

```
ValueError: 'os  # Use os.pathsep. Default configuration used for new projects.' is not a valid value for version_path_separator; expected 'space', 'os', ':', ';'
```

It seemed that the comment in the last line was being included as part of the parsed config value, which should be `os`.

Alembic currently [uses `configparser.ConfigParser` from the standard libary](https://github.com/sqlalchemy/alembic/blob/2d60c77c81a72a78b575b96aef511e658073dec5/alembic/util/compat.py#L82-L89) to parse `alembic.ini` files. The [default `configparser` file format](https://docs.python.org/3/library/configparser.html#supported-ini-file-structure) requires that comments be on their own lines, although this can be customized. I changed the three copies of this line in Alembic's `alembic.ini` templates to remove the inline comments. In my case, this change fixed the `ValueError`.

This issue could also be fixed by changing [the default instance of `ConfigParser`](https://github.com/sqlalchemy/alembic/blob/2d60c77c81a72a78b575b96aef511e658073dec5/alembic/config.py#L202), using `inline_comment_prefixes=("#",)`. I imagine, however, that it might be better to use the default file format.

### Checklist
<!-- 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:

- [x] 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.
- [ ] 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.

**Have a nice day!**

Closes: #1397
Pull-request: sqlalchemy/alembic#1397
Pull-request-sha: 073cbd9b076fffa78be3b374e20ece4a2562f7bd

Change-Id: I49bc76994617a3c17a2443a4e4387c2479a661a4
  • Loading branch information
zzzeek authored and sqla-tester committed Nov 6, 2024
1 parent 7fe422a commit b5337ea
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 3 deletions.
4 changes: 3 additions & 1 deletion alembic/templates/async/alembic.ini.mako
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,9 @@ prepend_sys_path = .
# version_path_separator = ;
# version_path_separator = space
# version_path_separator = newline
version_path_separator = os # Use os.pathsep. Default configuration used for new projects.
#
# Use os.pathsep. Default configuration used for new projects.
version_path_separator = os

# set to 'true' to search source files recursively
# in each "version_locations" directory
Expand Down
4 changes: 3 additions & 1 deletion alembic/templates/generic/alembic.ini.mako
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ prepend_sys_path = .
# version_path_separator = ;
# version_path_separator = space
# version_path_separator = newline
version_path_separator = os # Use os.pathsep. Default configuration used for new projects.
#
# Use os.pathsep. Default configuration used for new projects.
version_path_separator = os

# set to 'true' to search source files recursively
# in each "version_locations" directory
Expand Down
4 changes: 3 additions & 1 deletion alembic/templates/multidb/alembic.ini.mako
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,9 @@ prepend_sys_path = .
# version_path_separator = ;
# version_path_separator = space
# version_path_separator = newline
version_path_separator = os # Use os.pathsep. Default configuration used for new projects.
#
# Use os.pathsep. Default configuration used for new projects.
version_path_separator = os

# set to 'true' to search source files recursively
# in each "version_locations" directory
Expand Down

0 comments on commit b5337ea

Please sign in to comment.