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.

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

git does not commit "versions" by default, so we should possibly ensure some file is present there #1320

Open
sjsadowski opened this issue Oct 11, 2023 · 6 comments
Labels
migration environment use case not quite a feature and not quite a bug, something we just didn't think of

Comments

@sjsadowski
Copy link

Describe the bug
Creating a revision fails if the versions directory is missing

Expected behavior
versions directory is created, then revision is created or a clean error message explaining the problem

To Reproduce
Please try to provide a Minimal, Complete, and Verifiable example, with the migration script and/or the SQLAlchemy tables or models involved.
See also Reporting Bugs on the website.

(main)|pim]$ pdm run alembic -c alembic.dev.ini revision -m "create product table"

Error

  Generating /projects/pim/migrations/versions/b6966ad88634_create_product_table.py ...  FAILED
Traceback (most recent call last):
  File "/projects/pim/.venv/bin/alembic", line 10, in <module>
    sys.exit(main())
             ^^^^^^
  File "/projects/pim/.venv/lib/python3.11/site-packages/alembic/config.py", line 630, in main
    CommandLine(prog=prog).main(argv=argv)
  File "/projects/pim/.venv/lib/python3.11/site-packages/alembic/config.py", line 624, in main
    self.run_cmd(cfg, options)
  File "/projects/pim/.venv/lib/python3.11/site-packages/alembic/config.py", line 601, in run_cmd
    fn(
  File "/projects/pim/.venv/lib/python3.11/site-packages/alembic/command.py", line 240, in revision
    scripts = [script for script in revision_context.generate_scripts()]
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/projects/pim/.venv/lib/python3.11/site-packages/alembic/command.py", line 240, in <listcomp>
    scripts = [script for script in revision_context.generate_scripts()]
              ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/projects/pim/.venv/lib/python3.11/site-packages/alembic/autogenerate/api.py", line 640, in generate_scripts
    yield self._to_script(generated_revision)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/projects/pim/.venv/lib/python3.11/site-packages/alembic/autogenerate/api.py", line 545, in _to_script
    return self.script_directory.generate_revision(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/projects/pim/.venv/lib/python3.11/site-packages/alembic/script/base.py", line 740, in generate_revision
    self._generate_template(
  File "/projects/pim/.venv/lib/python3.11/site-packages/alembic/script/base.py", line 588, in _generate_template
    util.template_to_file(src, dest, self.output_encoding, **kw)
  File "/projects/pim/.venv/lib/python3.11/site-packages/alembic/util/pyfiles.py", line 39, in template_to_file
    with open(dest, "wb") as f:
         ^^^^^^^^^^^^^^^^
FileNotFoundError: [Errno 2] No such file or directory: '/projects/pim/migrations/versions/b6966ad88634_create_product_table.py'

Versions.

  • OS: MacOS 13.5.2 (22.6.0 Darwin Kernel Version 22.6.0: Wed Jul 5 22:22:05 PDT 2023; root:xnu-8796.141.3~6/RELEASE_ARM64_T6000 arm64)
  • Python: 3.11
  • Alembic: 1.12.0
  • SQLAlchemy: 2.0.21
  • Database: Postgres
  • DBAPI: asyncpg

Additional context
This is a corner case. I initialized alembic and due to the timing, committed after the initialization but before creating the first migration. The versions directory was empty and was not preserved. I moved to a different device and ran a pull, then tried to create the initial revision. That failed.

As you may note from the above, I am using pdm and initially thought there was a pathing issue. That is not the case - I activated the virtualenv and was able to reproduce.

Obviously this is a bit of a silly issue, as it's unlikely many people are going to initialize alembic and then not create an initial revision, but it could happen to someone else. I suspect that autoinstantiating the versions directory if not present is probably the easiest behavior to include, but if that can't happen for some reason (lacking create permissions/device full/other things) catching the issue and then producing a clear error message seems like the right thing.

Have a nice day!

@sjsadowski sjsadowski added the requires triage New issue that requires categorization label Oct 11, 2023
@zzzeek zzzeek changed the title creating revision fails if versions directory is missing git does not commit "versions" by default, so we should possibly ensure some file is present there Oct 11, 2023
@zzzeek zzzeek added migration environment use case not quite a feature and not quite a bug, something we just didn't think of and removed requires triage New issue that requires categorization labels Oct 11, 2023
@zzzeek
Copy link
Member

zzzeek commented Oct 11, 2023

I suspect that autoinstantiating the versions directory if not present is probably the easiest behavior to include,

I wouldnt want to do that. the alembic init call is where people expect this directory to be created. auto-creating it again is a bit of a surprise and suggests we shouldn't have "versions" in the first place, since it would be auto-created as needed.

the issue here is git's behavior of not including empty folders. The only real way to work around this is to have a file present in ./versions; right now this can be achieved using alembic init --package which will add an __init__.py file.

I would propose some other kind of file in the templates like a README.rst with a single line.

@CaselIT thoughts?

@CaselIT
Copy link
Member

CaselIT commented Oct 11, 2023

I wouldnt want to do that. the alembic init call is where people expect this directory to be created. auto-creating it again is a bit of a surprise and suggests we shouldn't have "versions" in the first place, since it would be auto-created as needed.

not sure, I can't think of a reason why someone would be surprised by it.
Alternatively adding a .gitkeep file on init could be an ok alternative that should not cause much surprise to users.

@zzzeek
Copy link
Member

zzzeek commented Oct 11, 2023

yah i googled a bit, is .gitkeep a convention of some kind ? seems to be used a lot, though then you see things like https://adamj.eu/tech/2023/09/18/git-dont-create-gitkeep/ . .gitkeep is only a convention, not recognized by git itself.

should we use the .,gitignore convention approach instead? I have seen that mentioned a few places.

@sjsadowski
Copy link
Author

sjsadowski commented Oct 11, 2023

should we use the .,gitignore convention approach instead? I have seen that mentioned a few places.

First: I don't particularly have a horse in the race myself other than uncovering the issue somewhat by mistake. To me it makes the most sense to have an empty __init__.py in the versions folder because alembic itself has no real responsibility to make sure git behaves (imho) so there's no real need for .gitignore or .gitkeep, just for alembic to accurately and cleanly tell the user what the problem is.

The idea of autoinstantiation was more about user experience, but it seems like "ERROR: versions directory does not exist" or something similar would be more than enough.

Anyway, thanks for jumping on this so quickly.

@zzzeek
Copy link
Member

zzzeek commented Oct 11, 2023

IIRC the __init__.py is also a surprise since it means various kinds of testing / discovery tools now see "versions" as a package, which might not be what people want.

@CaselIT
Copy link
Member

CaselIT commented Oct 11, 2023

yah i googled a bit, is .gitkeep a convention of some kind ? seems to be used a lot, though then you see things like https://adamj.eu/tech/2023/09/18/git-dont-create-gitkeep/ . .gitkeep is only a convention, not recognized by git itself.

should we use the .,gitignore convention approach instead? I have seen that mentioned a few places.

I don't think it applies in this case, since we aren't trying to ignore everything inside version.
But I think its fine also with an empty .gitignore.

I think .gitkeep is kind of a convention since it's clear what the scope of the file is (an empty gitignore is not clear why it's there) and for git its just another file.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
migration environment use case not quite a feature and not quite a bug, something we just didn't think of
Projects
None yet
Development

No branches or pull requests

3 participants