You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2021/12/01 14:16:46 UTC
[GitHub] [airflow] lwyszomi opened a new pull request #19932: Inrease length of the email and username
lwyszomi opened a new pull request #19932:
URL: https://github.com/apache/airflow/pull/19932
When we using service account as a `email` or `username` we can reach a character limit of these fields.
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/CONTRIBUTING.rst#pull-request-guidelines)** for more information.
In case of fundamental code change, Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvements+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in [UPDATING.md](https://github.com/apache/airflow/blob/main/UPDATING.md).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] swalkowski commented on a change in pull request #19932: Increase length of the email and username
Posted by GitBox <gi...@apache.org>.
swalkowski commented on a change in pull request #19932:
URL: https://github.com/apache/airflow/pull/19932#discussion_r761024484
##########
File path: docs/apache-airflow/migrations-ref.rst
##########
@@ -23,7 +23,9 @@ Here's the list of all the Database Migrations that are executed via when you ru
+--------------------------------+------------------+-----------------+---------------------------------------------------------------------------------------+
| Revision ID | Revises ID | Airflow Version | Description |
+--------------------------------+------------------+-----------------+---------------------------------------------------------------------------------------+
-| ``be2bfac3da23`` (head) | ``7b2661a43ba3`` | ``2.2.3`` | Add has_import_errors column to DagModel |
+| ``5e3ec427fdd3`` (head) | ``be2bfac3da23`` | ``2.2.3`` | Increase length of email and username in ``ab_user`` table to ``256`` characters |
Review comment:
Shouldn't we include changing column lengths in `ab_register_user` in this migration as well, given that you update `ab_register_user` table in `models.py`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19932: Increase length of the email and username
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19932:
URL: https://github.com/apache/airflow/pull/19932#discussion_r761036647
##########
File path: docs/apache-airflow/migrations-ref.rst
##########
@@ -23,7 +23,9 @@ Here's the list of all the Database Migrations that are executed via when you ru
+--------------------------------+------------------+-----------------+---------------------------------------------------------------------------------------+
| Revision ID | Revises ID | Airflow Version | Description |
+--------------------------------+------------------+-----------------+---------------------------------------------------------------------------------------+
-| ``be2bfac3da23`` (head) | ``7b2661a43ba3`` | ``2.2.3`` | Add has_import_errors column to DagModel |
+| ``5e3ec427fdd3`` (head) | ``be2bfac3da23`` | ``2.2.3`` | Increase length of email and username in ``ab_user`` table to ``256`` characters |
Review comment:
👍
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] swalkowski commented on a change in pull request #19932: Increase length of the email and username
Posted by GitBox <gi...@apache.org>.
swalkowski commented on a change in pull request #19932:
URL: https://github.com/apache/airflow/pull/19932#discussion_r761005118
##########
File path: airflow/migrations/versions/5e3ec427fdd3_increase_length_of_email_and_username.py
##########
@@ -0,0 +1,48 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Increase length of email and username
+
+Revision ID: 5e3ec427fdd3
+Revises: be2bfac3da23
+Create Date: 2021-12-01 11:49:26.390210
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = '5e3ec427fdd3'
+down_revision = 'be2bfac3da23'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+ """Increase length of email from 64 to 256 characters"""
+ with op.batch_alter_table('ab_user') as batch_op:
+ batch_op.alter_column('username', type_=sa.String(256))
+ batch_op.alter_column('email', type_=sa.String(256))
+
+
+def downgrade():
+ """Revert length of email from 256 to 64 characters"""
+ with op.batch_alter_table('user') as batch_op:
Review comment:
Should be `ab_user` here as well?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk merged pull request #19932: Increase length of the email and username
Posted by GitBox <gi...@apache.org>.
potiuk merged pull request #19932:
URL: https://github.com/apache/airflow/pull/19932
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on a change in pull request #19932: Increase length of the email and username
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on a change in pull request #19932:
URL: https://github.com/apache/airflow/pull/19932#discussion_r761013582
##########
File path: airflow/migrations/versions/5e3ec427fdd3_increase_length_of_email_and_username.py
##########
@@ -0,0 +1,48 @@
+#
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Increase length of email and username
+
+Revision ID: 5e3ec427fdd3
+Revises: be2bfac3da23
+Create Date: 2021-12-01 11:49:26.390210
+
+"""
+
+import sqlalchemy as sa
+from alembic import op
+
+# revision identifiers, used by Alembic.
+revision = '5e3ec427fdd3'
+down_revision = 'be2bfac3da23'
+branch_labels = None
+depends_on = None
+
+
+def upgrade():
+ """Increase length of email from 64 to 256 characters"""
+ with op.batch_alter_table('ab_user') as batch_op:
+ batch_op.alter_column('username', type_=sa.String(256))
+ batch_op.alter_column('email', type_=sa.String(256))
+
+
+def downgrade():
+ """Revert length of email from 256 to 64 characters"""
+ with op.batch_alter_table('user') as batch_op:
Review comment:
@swalkowski 🦅 👁️
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] lwyszomi commented on pull request #19932: Increase length of the email and username
Posted by GitBox <gi...@apache.org>.
lwyszomi commented on pull request #19932:
URL: https://github.com/apache/airflow/pull/19932#issuecomment-984360683
@uranusjr `ID_LEN` for me should be used only for id fields to keep nameing convention clear.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] uranusjr commented on pull request #19932: Increase length of the email and username
Posted by GitBox <gi...@apache.org>.
uranusjr commented on pull request #19932:
URL: https://github.com/apache/airflow/pull/19932#issuecomment-984305066
Instead of changing an arbitrary length to another, it may be better to use `ID_LEN` in `airflow.models.base`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] potiuk commented on pull request #19932: Increase length of the email and username
Posted by GitBox <gi...@apache.org>.
potiuk commented on pull request #19932:
URL: https://github.com/apache/airflow/pull/19932#issuecomment-986312717
Only internittent errors that are being fixed already. Merging
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [airflow] github-actions[bot] commented on pull request #19932: Increase length of the email and username
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #19932:
URL: https://github.com/apache/airflow/pull/19932#issuecomment-983984495
The PR most likely needs to run full matrix of tests because it modifies parts of the core of Airflow. However, committers might decide to merge it quickly and take the risk. If they don't merge it quickly - please rebase it to the latest main at your convenience, or amend the last commit of the PR, and push it with --force-with-lease.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org