Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

prepend_sys_path does not work with absolute paths on windows #1330

Open
mwerezak opened this issue Oct 19, 2023 · 7 comments · May be fixed by #1331
Open

prepend_sys_path does not work with absolute paths on windows #1330

mwerezak opened this issue Oct 19, 2023 · 7 comments · May be fixed by #1331
Labels
bug Something isn't working configuration PRs (with tests!) welcome

Comments

@mwerezak
Copy link

mwerezak commented Oct 19, 2023 •
edited

Describe the bug

If you put an absolute path in prepend_sys_path in your alembic.ini on windows (which will happen if you use %(here)s), the path will get split on the ':' character that separates the drive letter.

Expected behavior

Absolute paths to not get broken.

To Reproduce
Use an absolute path in prepend_sys_path like C:/something or %(here)s/something

Then print out sys.path, e.g. for path in sys.path: print(path) you get:

C
\something
C
\path\to\here\something

# Insert code here

Error
ModuleNotFoundError: No module named ...
because env.py can't import my modules.

Versions.

  • OS: Windows 10
  • Python: 3.11.5
  • Alembic: 1.12.0
  • SQLAlchemy: 1.4.49
  • Database: Postgres
  • DBAPI: psycopg2

Additional context

alembic/alembic/script/base.py

Lines 212 to 218 in 3e48ed0

prepend_sys_path = config.get_main_option("prepend_sys_path")
if prepend_sys_path:
sys.path[:0] = list(
_split_on_space_comma_colon.split(prepend_sys_path)
)

@mwerezak mwerezak added the requires triage New issue that requires categorization label Oct 19, 2023
@zzzeek
Copy link
Member

zzzeek commented Oct 19, 2023

@CaselIT is this also #1284 ?

@zzzeek zzzeek added bug Something isn't working configuration and removed requires triage New issue that requires categorization labels Oct 19, 2023
@zzzeek
Copy link
Member

zzzeek commented Oct 19, 2023

PRs with tests welcome here

mwerezak added a commit to mwerezak/alembic that referenced this issue Oct 19, 2023
@mwerezak
Improve handling of prepend_sys_path
a4b3782 
Don't split using colons on windows
Fixes sqlalchemy#1330
@CaselIT
Copy link
Member

CaselIT commented Oct 19, 2023

@CaselIT is this also #1284 ?

it should not, since that depend on the value of version_path_separator

@zzzeek
Copy link
Member

zzzeek commented Oct 19, 2023

Proposed in #1331 we add a new config value with legacy fallback behavior as follows:

# sys.path path, will be prepended to sys.path if present.
# defaults to the current working directory.  for multiple paths, the path separator
# is defined by prepend_sys_path_separator
prepend_sys_path = .

# prepend sys path separator; As mentioned above, this is the character used to split
# sys_path paths.  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 prepend_sys_path_separator are:
#
# prepend_sys_path_separator = :
# prepend_sys_path_separator = ;
# prepend_sys_path_separator = space
prepend_sys_path_separator = os  # Use os.pathsep. Default configuration used for new projects.

@CaselIT
Copy link
Member

CaselIT commented Oct 19, 2023

it's probably the most sensible option. it makes little sense to have the two paths in the config handled in a different way

@zzzeek
Copy link
Member

zzzeek commented Oct 19, 2023

I guess if we were on pyproject.toml this whole thing would be a non issue since it supports lists :)

@CaselIT
Copy link
Member

CaselIT commented Oct 19, 2023

does not support %(here)s though

@CaselIT CaselIT linked a pull request Nov 22, 2023 that will close this issue
Open
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working configuration PRs (with tests!) welcome
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Improve handling of prepend_sys_path, fixes #1330 mwerezak/alembic
3 participants
@zzzeek @mwerezak @CaselIT