sqlalchemy / alembic

A database migrations tool for SQLAlchemy.
MIT License
2.76k stars 241 forks source link

Fix `alembic.ini` templates to match `configparser` file format. #1397

Open bhushan-mohanraj opened 8 months ago

bhushan-mohanraj commented 8 months ago

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 to parse alembic.ini files. The default configparser file format 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, using inline_comment_prefixes=("#",). I imagine, however, that it might be better to use the default file format.

Checklist

This pull request is:

Have a nice day!

CaselIT commented 8 months ago

Thanks

Seems to be a reasonable change