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 2022/10/25 15:46:08 UTC
[GitHub] [airflow] ephraimbuddy opened a new pull request, #27266: Don't lowercase username when looking up user
ephraimbuddy opened a new pull request, #27266:
URL: https://github.com/apache/airflow/pull/27266
Since usernames can be a combination of upper and lower cases. We shouldn't lower the case when searching for a username
--
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] ephraimbuddy commented on a diff in pull request #27266: Add case insensitive constraint to username
Posted by GitBox <gi...@apache.org>.
ephraimbuddy commented on code in PR #27266:
URL: https://github.com/apache/airflow/pull/27266#discussion_r1005532101
##########
airflow/utils/db.py:
##########
@@ -930,6 +930,36 @@ def check_conn_id_duplicates(session: Session) -> Iterable[str]:
)
+def check_username_duplicates(session: Session) -> Iterable[str]:
+ """
+ Check unique username in User & RegisterUser table
+
+ :param session: session of the sqlalchemy
+ :rtype: str
+ """
+ from airflow.www.fab_security.sqla.models import RegisterUser, User
+
+ for model in [User, RegisterUser]:
+ dups = []
+ try:
+ dups = (
+ session.query(model.username) # type: ignore[attr-defined]
+ .group_by(model.username) # type: ignore[attr-defined]
+ .having(func.count() > 1)
+ .all()
+ )
+ except (exc.OperationalError, exc.ProgrammingError):
+ # fallback if tables hasn't been created yet
+ session.rollback()
+ if dups:
+ yield (
+ f'Seems you have non unique username in {model.__table__.name} table.\n' # type: ignore
+ 'You have to manage those duplicate usernames '
Review Comment:
Addressed. You can take another look. Thanks
--
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] ashb commented on a diff in pull request #27266: Add case insensitive constraint to username
Posted by GitBox <gi...@apache.org>.
ashb commented on code in PR #27266:
URL: https://github.com/apache/airflow/pull/27266#discussion_r1005465343
##########
airflow/utils/db.py:
##########
@@ -930,6 +930,36 @@ def check_conn_id_duplicates(session: Session) -> Iterable[str]:
)
+def check_username_duplicates(session: Session) -> Iterable[str]:
+ """
+ Check unique username in User & RegisterUser table
+
+ :param session: session of the sqlalchemy
+ :rtype: str
+ """
+ from airflow.www.fab_security.sqla.models import RegisterUser, User
+
+ for model in [User, RegisterUser]:
+ dups = []
+ try:
+ dups = (
+ session.query(model.username) # type: ignore[attr-defined]
+ .group_by(model.username) # type: ignore[attr-defined]
+ .having(func.count() > 1)
+ .all()
+ )
+ except (exc.OperationalError, exc.ProgrammingError):
+ # fallback if tables hasn't been created yet
+ session.rollback()
+ if dups:
+ yield (
+ f'Seems you have non unique username in {model.__table__.name} table.\n' # type: ignore
+ 'You have to manage those duplicate usernames '
Review Comment:
This needs to say something about mixed-case, as otherwise users could be forgiven for thinking that `Test` and `test` are different users.
And "manage" isn't clear either -> "delete or rename"
--
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] ephraimbuddy merged pull request #27266: Add case insensitive constraint to username
Posted by GitBox <gi...@apache.org>.
ephraimbuddy merged PR #27266:
URL: https://github.com/apache/airflow/pull/27266
--
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 a diff in pull request #27266: Add case insensitive constraint to username
Posted by GitBox <gi...@apache.org>.
potiuk commented on code in PR #27266:
URL: https://github.com/apache/airflow/pull/27266#discussion_r1006549764
##########
airflow/utils/db.py:
##########
@@ -929,6 +929,36 @@ def check_conn_id_duplicates(session: Session) -> Iterable[str]:
)
+def check_username_duplicates(session: Session) -> Iterable[str]:
+ """
+ Check unique username in User & RegisterUser table
+
+ :param session: session of the sqlalchemy
+ :rtype: str
+ """
+ from airflow.www.fab_security.sqla.models import RegisterUser, User
+
+ for model in [User, RegisterUser]:
+ dups = []
+ try:
+ dups = (
+ session.query(model.username) # type: ignore[attr-defined]
+ .group_by(model.username) # type: ignore[attr-defined]
+ .having(func.count() > 1)
+ .all()
+ )
+ except (exc.OperationalError, exc.ProgrammingError):
+ # fallback if tables hasn't been created yet
+ session.rollback()
+ if dups:
+ yield (
+ f'Seems you have mixed case usernames in {model.__table__.name} table.\n' # type: ignore
+ 'You have to rename or delete those mixed case usernames '
Review Comment:
:heart: love the detailed error message
--
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