-
Notifications
You must be signed in to change notification settings - Fork 14.2k
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
Fix alembic template & make pre-commit more resilient #43244
Merged
Merged
+5
−5
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
ashb
approved these changes
Oct 22, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could also get alembic to ruff format things automatically
Lee-W
approved these changes
Oct 22, 2024
While working on this apache#43243, I was following https://github.com/apache/airflow/blob/main/contributing-docs/13_metadata_database_updates.rst and I ran into an error with pre-commit hook. When running the revision command as follows: ``` root@f1f78138ad78:/opt/airflow/airflow# alembic revision -m "New revision" Generating /opt/airflow/airflow/migrations/versions/01b38be821e9_new_revision.py ... done ``` It creates a file as follows: ```python """New revision Revision ID: cd7be1ae8b80 Revises: 05234396c6fc Create Date: 2024-10-22 01:44:17.873864 """ import sqlalchemy as sa from alembic import op # revision identifiers, used by Alembic. revision = 'cd7be1ae8b80' down_revision = '05234396c6fc' branch_labels = None depends_on = None ``` Notice single quotes in `revision` & `down_revision`. Now if I just run that single pre-commit hook (`update-migration-references`), it fails ``` ❯ pre-commit run "update-migration-references" --all-files Update migration ref doc.................................................Failed - hook id: update-migration-references - exit code: 1 Using 'uv' to install Airflow Using airflow version from current sources Updating migration reference for airflow Making sure airflow version updated Making sure there's no mismatching revision numbers Traceback (most recent call last): File "/opt/airflow/scripts/in_container/run_migration_reference.py", line 246, in <module> correct_mismatching_revision_nums(revisions=revisions) File "/opt/airflow/scripts/in_container/run_migration_reference.py", line 230, in correct_mismatching_revision_nums new_content = content.replace(revision_id_match.group(1), revision_match.group(1), 1) AttributeError: 'NoneType' object has no attribute 'group' Error 1 returned If you see strange stacktraces above, run `breeze ci-image build --python 3.9` and try again. ``` That isn't a problem generally as `ruff` will fail before and convert it into double quotes. But rather than doing that, we fix it at source.
kaxil
force-pushed
the
alembic-template
branch
from
October 22, 2024 10:36
e3e4171
to
53c6ec5
Compare
harjeevanmaan
pushed a commit
to harjeevanmaan/airflow
that referenced
this pull request
Oct 23, 2024
While working on this apache#43243, I was following https://github.com/apache/airflow/blob/main/contributing-docs/13_metadata_database_updates.rst and I ran into an error with pre-commit hook. When running the revision command as follows: ``` root@f1f78138ad78:/opt/airflow/airflow# alembic revision -m "New revision" Generating /opt/airflow/airflow/migrations/versions/01b38be821e9_new_revision.py ... done ``` It creates a file as follows: ```python """New revision Revision ID: cd7be1ae8b80 Revises: 05234396c6fc Create Date: 2024-10-22 01:44:17.873864 """ import sqlalchemy as sa from alembic import op # revision identifiers, used by Alembic. revision = 'cd7be1ae8b80' down_revision = '05234396c6fc' branch_labels = None depends_on = None ``` Notice single quotes in `revision` & `down_revision`. Now if I just run that single pre-commit hook (`update-migration-references`), it fails ``` ❯ pre-commit run "update-migration-references" --all-files Update migration ref doc.................................................Failed - hook id: update-migration-references - exit code: 1 Using 'uv' to install Airflow Using airflow version from current sources Updating migration reference for airflow Making sure airflow version updated Making sure there's no mismatching revision numbers Traceback (most recent call last): File "/opt/airflow/scripts/in_container/run_migration_reference.py", line 246, in <module> correct_mismatching_revision_nums(revisions=revisions) File "/opt/airflow/scripts/in_container/run_migration_reference.py", line 230, in correct_mismatching_revision_nums new_content = content.replace(revision_id_match.group(1), revision_match.group(1), 1) AttributeError: 'NoneType' object has no attribute 'group' Error 1 returned If you see strange stacktraces above, run `breeze ci-image build --python 3.9` and try again. ``` That isn't a problem generally as `ruff` will fail before and convert it into double quotes. But rather than doing that, we fix it at source.
PaulKobow7536
pushed a commit
to PaulKobow7536/airflow
that referenced
this pull request
Oct 24, 2024
While working on this apache#43243, I was following https://github.com/apache/airflow/blob/main/contributing-docs/13_metadata_database_updates.rst and I ran into an error with pre-commit hook. When running the revision command as follows: ``` root@f1f78138ad78:/opt/airflow/airflow# alembic revision -m "New revision" Generating /opt/airflow/airflow/migrations/versions/01b38be821e9_new_revision.py ... done ``` It creates a file as follows: ```python """New revision Revision ID: cd7be1ae8b80 Revises: 05234396c6fc Create Date: 2024-10-22 01:44:17.873864 """ import sqlalchemy as sa from alembic import op # revision identifiers, used by Alembic. revision = 'cd7be1ae8b80' down_revision = '05234396c6fc' branch_labels = None depends_on = None ``` Notice single quotes in `revision` & `down_revision`. Now if I just run that single pre-commit hook (`update-migration-references`), it fails ``` ❯ pre-commit run "update-migration-references" --all-files Update migration ref doc.................................................Failed - hook id: update-migration-references - exit code: 1 Using 'uv' to install Airflow Using airflow version from current sources Updating migration reference for airflow Making sure airflow version updated Making sure there's no mismatching revision numbers Traceback (most recent call last): File "/opt/airflow/scripts/in_container/run_migration_reference.py", line 246, in <module> correct_mismatching_revision_nums(revisions=revisions) File "/opt/airflow/scripts/in_container/run_migration_reference.py", line 230, in correct_mismatching_revision_nums new_content = content.replace(revision_id_match.group(1), revision_match.group(1), 1) AttributeError: 'NoneType' object has no attribute 'group' Error 1 returned If you see strange stacktraces above, run `breeze ci-image build --python 3.9` and try again. ``` That isn't a problem generally as `ruff` will fail before and convert it into double quotes. But rather than doing that, we fix it at source.
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.
While working on this #43243, I was following https://github.com/apache/airflow/blob/main/contributing-docs/13_metadata_database_updates.rst and I ran into an error with pre-commit hook.
When running the revision command as follows:
It creates a file as follows:
Notice single quotes in
revision
&down_revision
.Now if I just run that single pre-commit hook (
update-migration-references
), it failsThat isn't a problem generally as
ruff
will fail before and convert it into double quotes. But rather than doing that, we fix it at source.^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rst
or{issue_number}.significant.rst
, in newsfragments.