-
Notifications
You must be signed in to change notification settings - Fork 4
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
Try out Alembic #82
Draft
gnn
wants to merge
13
commits into
dev
Choose a base branch
from
features/#81-use-alembic-to-version-db-structure
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Try out Alembic #82
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Except for some style fixes, these files where generated by running `alembic init alembic` inside "src/egon/data".
This is also the result of running `alembic init alembic`, but I forgot to add the file in the previous commit, because I only committed the contents of the alembic directory, whereas the "alembic.ini" file got placed directly outside of it.
This includes only ORMs for target tables in schema openstreetmap
Just using the revision id doesn't convey too much information about the script when looking at the filename. It especially doesn't convey any information with respect to the relative ordering among scripts. Adding the time stamp to the script's filename at least gives an idea about this and it also gives a hint on how recent the change is. One even might use this information to figure how whether old migrations should be pruned in the future. When one talks about timestamps, it's of course good to always have them in UTC. That way, they are comparable even without specifying time zone information, which would just add unnecessary noise here. I also changed the character separating the different parts of the filename from underscores to periods, because those are already used to separate the suffix from the rest of the filename and using a smaller number if different characters is easier on the eyes, IMHO. Last but not least, I relaxed the length at which the revision message to 53 characters. This is the maximum length of the first line of a Git commit message, i.e. 50 characters, with a little bit of leeway.
Since we use `black` and `isort` to ensure a consistent style in our Python code, running newly generated migration scripts through them ensures that not code violating our style guide gets automatically generated. Note that using Alembic's `console_scripts` entry point type unfortunately only works with Python console scripts installed in the same virtual environment as the `alembic` in use. If that starts to become a problem, we have to implement a custom write hook in "env.py" which calls arbitrary external programs.
Since this moves the migration script, this hook has to be run after every other hook. Everything else should be explained in the comments and docstrings. We also have to set `revision_environment` to `true` because the "env.py" file contains our hook definition which always has to be loaded in order to move a generated migration script.
The file was generated using the command ``` alembic revision -m "Do nothing for the base migration" ``` inside the "src/egon/data/alembic" directory. This is important, as this directory contains the "alembic.ini" file which Alembic needs to operate. As the docstring states, this migration is initially empty in order to provide a blank slate to build upon and to go back to.
The extensions in question are hstore and postgis. These are currently added via a Docker entry point. Since this already created problems and since it also wouldn't work when not using Docker, moving it into a migration is both, a good proof of concept and a fix for an open issue.
Since the migrations currently also contain one that creates the database extensions we need, this also fixes #76 and the Docker entry point is removed with this commit.
The script was automatically generated by setting `target_metadata` in "src/egon/data/alembic/env.py" to `egon.data.orm.openstreetmap.metadata` and running ``` alembic revision \ --autogenerate \ -m "Create the OpenStreetMap database structure" ``` The script needs some adjustments though, which I'm putting into a separate commit for documentation purposes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
In order to figure out, whether the benefits of using Alembic outweigh the development costs, we have to try it out. The commits in this pull request are us doing so.
This addresses issue #81.