From cb3e82011a8036f6ed032ad03f1b3277e356fb14 Mon Sep 17 00:00:00 2001 From: Pat Garziglia <49124058+patgarz@users.noreply.github.com> Date: Fri, 1 Sep 2023 13:34:47 -0400 Subject: [PATCH 01/11] Add get_user_display_name to AIP-56 base auth_manager --- airflow/auth/managers/base_auth_manager.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/airflow/auth/managers/base_auth_manager.py b/airflow/auth/managers/base_auth_manager.py index 66b79b0a26daf..d626f3681031a 100644 --- a/airflow/auth/managers/base_auth_manager.py +++ b/airflow/auth/managers/base_auth_manager.py @@ -54,6 +54,10 @@ def get_cli_commands() -> list[CLICommand]: def get_user_name(self) -> str: """Return the username associated to the user in session.""" + @abstractmethod + def get_user_display_name(self) -> str: + """Return the user's display name associated to the user in session.""" + @abstractmethod def get_user(self) -> BaseUser: """Return the user associated to the user in session.""" From e2e046477cd7a06d4fde1e96025b331754e21792 Mon Sep 17 00:00:00 2001 From: Pat Garziglia <49124058+patgarz@users.noreply.github.com> Date: Fri, 1 Sep 2023 13:38:52 -0400 Subject: [PATCH 02/11] Implement invariant, unique identifier for get_user_name in fab_auth_manager --- airflow/auth/managers/fab/fab_auth_manager.py | 20 ++++++--- .../templates/appbuilder/navbar_right.html | 10 ++--- .../managers/fab/test_fab_auth_manager.py | 41 ++++++++++++++++--- 3 files changed, 56 insertions(+), 15 deletions(-) diff --git a/airflow/auth/managers/fab/fab_auth_manager.py b/airflow/auth/managers/fab/fab_auth_manager.py index 27c96d6dd596c..4816da6b12270 100644 --- a/airflow/auth/managers/fab/fab_auth_manager.py +++ b/airflow/auth/managers/fab/fab_auth_manager.py @@ -62,17 +62,27 @@ def get_cli_commands() -> list[CLICommand]: SYNC_PERM_COMMAND, # not in a command group ] + def get_user_display_name(self) -> str: + """Return the user's display name associated to the user in session.""" + user = self.get_user() + first_name = user.first_name or "" + last_name = user.last_name or "" + return f"{first_name} {last_name}".strip() + def get_user_name(self) -> str: """ Return the username associated to the user in session. - For backward compatibility reasons, the username in FAB auth manager is the concatenation of the - first name and the last name. + For backward compatibility reasons, the username in FAB auth manager can be any of username, + email, or the database user ID. """ user = self.get_user() - first_name = user.first_name or "" - last_name = user.last_name or "" - return f"{first_name} {last_name}".strip() + if user.username: + return user.username + elif user.email: + return user.email + else: + return self.get_user_id() def get_user(self) -> User: """Return the user associated to the user in session.""" diff --git a/airflow/www/templates/appbuilder/navbar_right.html b/airflow/www/templates/appbuilder/navbar_right.html index 1ccf28db418f1..4227d7a3cc98a 100644 --- a/airflow/www/templates/appbuilder/navbar_right.html +++ b/airflow/www/templates/appbuilder/navbar_right.html @@ -66,11 +66,11 @@ {% if auth_manager.is_logged_in() %}
  • - - {% set user_name = auth_manager.get_user_name() %} - {% if user_name %} - {% set user_names = user_name.split(" ", 1) %} - {% for name in user_names %}{{ name[0].upper() }}{% endfor %} + + {% set user_display_name = auth_manager.get_user_display_name() %} + {% if user_display_name %} + {% set user_display_names = user_display_name.split(" ", 1) %} + {% for name in user_display_names %}{{ name[0].upper() }}{% endfor %} {% else %} person {% endif %} diff --git a/tests/auth/managers/fab/test_fab_auth_manager.py b/tests/auth/managers/fab/test_fab_auth_manager.py index 4802b23cf253a..595b6f9db6e8b 100644 --- a/tests/auth/managers/fab/test_fab_auth_manager.py +++ b/tests/auth/managers/fab/test_fab_auth_manager.py @@ -39,22 +39,53 @@ def auth_manager(): class TestFabAuthManager: @pytest.mark.parametrize( - "first_name,last_name,expected", + "id,first_name,last_name,username,email,expected", [ - ("First", "Last", "First Last"), - ("First", None, "First"), - (None, "Last", "Last"), + (1, "First", "Last", None, None, 1), + (1, None, None, None, None, 1), + (1, "First", "Last", "user", None, "user"), + (1, "First", "Last", "user", "email", "user"), + (1, None, None, None, "email", "email"), + (1, "First", "Last", None, "email", "email"), ], ) @mock.patch.object(FabAuthManager, "get_user") - def test_get_user_name(self, mock_get_user, first_name, last_name, expected, auth_manager): + def test_get_user_name( + self, mock_get_user, id, first_name, last_name, username, email, expected, auth_manager + ): user = User() + user.id = id user.first_name = first_name user.last_name = last_name + user.username = username + user.email = email mock_get_user.return_value = user assert auth_manager.get_user_name() == expected + @pytest.mark.parametrize( + "id,first_name,last_name,username,email,expected", + [ + (1, "First", "Last", None, None, "First Last"), + (1, "First", None, "user", None, "First"), + (1, None, "Last", "user", "email", "Last"), + (1, None, None, None, "email", None), + ], + ) + @mock.patch.object(FabAuthManager, "get_user") + def test_get_user_display_name( + self, mock_get_user, id, first_name, last_name, username, email, expected, auth_manager + ): + user = User() + user.id = id + user.first_name = first_name + user.last_name = last_name + user.username = username + user.email = email + mock_get_user.return_value = user + + assert auth_manager.get_user_display_name() == expected + @mock.patch("flask_login.utils._get_user") def test_get_user(self, mock_current_user, auth_manager): user = Mock() From 19e77a5b4eed4d3609a32d855ac2adf7e0f75a7f Mon Sep 17 00:00:00 2001 From: Pat Garziglia <49124058+patgarz@users.noreply.github.com> Date: Fri, 1 Sep 2023 15:55:36 -0400 Subject: [PATCH 03/11] Add friendly_name to Log table --- ...0_add_friendly_owner_to_audit_log_table.py | 50 + airflow/models/log.py | 6 +- airflow/utils/db.py | 2 +- airflow/www/decorators.py | 3 + airflow/www/views.py | 25 +- docs/apache-airflow/img/airflow_erd.sha256 | 2 +- docs/apache-airflow/img/airflow_erd.svg | 2670 ++++++++--------- docs/apache-airflow/migrations-ref.rst | 4 +- 8 files changed, 1415 insertions(+), 1347 deletions(-) create mode 100644 airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py diff --git a/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py b/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py new file mode 100644 index 0000000000000..6a2b3ac79bc3d --- /dev/null +++ b/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py @@ -0,0 +1,50 @@ +# +# 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. + +"""Add friendly_owner to (Audit) Log table + +Revision ID: f7bf2a57d0a6 +Revises: 375a816bbbf4 +Create Date: 2023-09-12 17:21:45.149658 + +""" + +import sqlalchemy as sa +from alembic import op + + +# revision identifiers, used by Alembic. +revision = "f7bf2a57d0a6" +down_revision = "375a816bbbf4" +branch_labels = None +depends_on = None +airflow_version = "2.8.0" + +TABLE_NAME = "log" + + +def upgrade(): + """Adds owner_display_name column to log""" + with op.batch_alter_table(TABLE_NAME) as batch_op: + batch_op.add_column(sa.Column("owner_display_name", sa.String(500))) + + +def downgrade(): + """Removes owner_display_name column from log""" + with op.batch_alter_table(TABLE_NAME) as batch_op: + batch_op.drop_column("owner_display_name") diff --git a/airflow/models/log.py b/airflow/models/log.py index c3cbaeeabc3cf..adc4476417ea1 100644 --- a/airflow/models/log.py +++ b/airflow/models/log.py @@ -37,6 +37,7 @@ class Log(Base): event = Column(String(30)) execution_date = Column(UtcDateTime) owner = Column(String(500)) + friendly_owner = Column(String(500)) extra = Column(Text) __table_args__ = ( @@ -45,7 +46,7 @@ class Log(Base): Index("idx_log_event", event), ) - def __init__(self, event, task_instance=None, owner=None, extra=None, **kwargs): + def __init__(self, event, task_instance=None, owner=None, friendly_owner=None, extra=None, **kwargs): self.dttm = timezone.utcnow() self.event = event self.extra = extra @@ -70,6 +71,7 @@ def __init__(self, event, task_instance=None, owner=None, extra=None, **kwargs): self.map_index = kwargs["map_index"] self.owner = owner or task_owner + self.friendly_owner = friendly_owner or None def __str__(self) -> str: - return f"Log({self.event}, {self.task_id}, {self.owner}, {self.extra})" + return f"Log({self.event}, {self.task_id}, {self.owner}, {self.friendly_owner}, {self.extra})" diff --git a/airflow/utils/db.py b/airflow/utils/db.py index f5c5313aa3f1f..9108ec2e5946f 100644 --- a/airflow/utils/db.py +++ b/airflow/utils/db.py @@ -88,7 +88,7 @@ "2.6.0": "98ae134e6fff", "2.6.2": "c804e5c76e3e", "2.7.0": "405de8318b3a", - "2.8.0": "375a816bbbf4", + "2.8.0": "f7bf2a57d0a6", } diff --git a/airflow/www/decorators.py b/airflow/www/decorators.py index 38aed59de94d0..9469c562b36ca 100644 --- a/airflow/www/decorators.py +++ b/airflow/www/decorators.py @@ -88,8 +88,10 @@ def wrapper(*args, **kwargs): with create_session() as session: if not get_auth_manager().is_logged_in(): user = "anonymous" + user_display = None else: user = get_auth_manager().get_user_name() + user_display = get_auth_manager().get_user_display_name() fields_skip_logging = {"csrf_token", "_csrf_token"} extra_fields = [ @@ -108,6 +110,7 @@ def wrapper(*args, **kwargs): event=event or f.__name__, task_instance=None, owner=user, + friendly_owner=user_display, extra=str(extra_fields), task_id=params.get("task_id"), dag_id=params.get("dag_id"), diff --git a/airflow/www/views.py b/airflow/www/views.py index 095f2b341095e..1e5dfa66f2c42 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -5495,11 +5495,32 @@ class LogModelView(AirflowModelView): permissions.ACTION_CAN_ACCESS_MENU, ] - list_columns = ["id", "dttm", "dag_id", "task_id", "event", "execution_date", "owner", "extra"] - search_columns = ["dttm", "dag_id", "task_id", "event", "execution_date", "owner", "extra"] + list_columns = [ + "id", + "dttm", + "dag_id", + "task_id", + "event", + "execution_date", + "owner", + "friendly_owner", + "extra", + ] + search_columns = [ + "dttm", + "dag_id", + "task_id", + "event", + "execution_date", + "owner", + "friendly_owner", + "extra", + ] label_columns = { "execution_date": "Logical Date", + "owner": "Owner ID", + "friendly_owner": "Owner Name", } base_order = ("dttm", "desc") diff --git a/docs/apache-airflow/img/airflow_erd.sha256 b/docs/apache-airflow/img/airflow_erd.sha256 index 5d5f1e453e267..cda5faa6048b1 100644 --- a/docs/apache-airflow/img/airflow_erd.sha256 +++ b/docs/apache-airflow/img/airflow_erd.sha256 @@ -1 +1 @@ -67b3dfeba9f0f4721ec4eaf3045ede333fdf16b40e6eaaa58f680f1dc95fcc6f \ No newline at end of file +1c67d53d5e407f0f78745764892ec7ff3dc02fb60fa89c4fda32129f369b97a4 \ No newline at end of file diff --git a/docs/apache-airflow/img/airflow_erd.svg b/docs/apache-airflow/img/airflow_erd.svg index d57ebdf1defd3..1aa4fb1b708c8 100644 --- a/docs/apache-airflow/img/airflow_erd.svg +++ b/docs/apache-airflow/img/airflow_erd.svg @@ -4,1628 +4,1618 @@ - - + + %3 - + ab_permission - -ab_permission - -id - [INTEGER] - NOT NULL - -name - [VARCHAR(100)] - NOT NULL + +ab_permission + +id + [INTEGER] + NOT NULL + +name + [VARCHAR(100)] + NOT NULL ab_permission_view - -ab_permission_view - -id - [INTEGER] - NOT NULL - -permission_id - [INTEGER] - -view_menu_id - [INTEGER] + +ab_permission_view + +id + [INTEGER] + NOT NULL + +permission_id + [INTEGER] + +view_menu_id + [INTEGER] ab_permission--ab_permission_view - -0..N -{0,1} + +0..N +{0,1} ab_permission_view_role - -ab_permission_view_role - -id - [INTEGER] - NOT NULL - -permission_view_id - [INTEGER] - -role_id - [INTEGER] + +ab_permission_view_role + +id + [INTEGER] + NOT NULL + +permission_view_id + [INTEGER] + +role_id + [INTEGER] ab_permission_view--ab_permission_view_role - -0..N -{0,1} + +0..N +{0,1} ab_view_menu - -ab_view_menu - -id - [INTEGER] - NOT NULL - -name - [VARCHAR(250)] - NOT NULL + +ab_view_menu + +id + [INTEGER] + NOT NULL + +name + [VARCHAR(250)] + NOT NULL ab_view_menu--ab_permission_view - -0..N -{0,1} + +0..N +{0,1} ab_role - -ab_role - -id - [INTEGER] - NOT NULL - -name - [VARCHAR(64)] - NOT NULL + +ab_role + +id + [INTEGER] + NOT NULL + +name + [VARCHAR(64)] + NOT NULL ab_role--ab_permission_view_role - -0..N -{0,1} + +0..N +{0,1} ab_user_role - -ab_user_role - -id - [INTEGER] - NOT NULL - -role_id - [INTEGER] - -user_id - [INTEGER] + +ab_user_role + +id + [INTEGER] + NOT NULL + +role_id + [INTEGER] + +user_id + [INTEGER] ab_role--ab_user_role - -0..N -{0,1} + +0..N +{0,1} ab_register_user - -ab_register_user - -id - [INTEGER] - NOT NULL - -email - [VARCHAR(512)] - NOT NULL - -first_name - [VARCHAR(256)] - NOT NULL - -last_name - [VARCHAR(256)] - NOT NULL - -password - [VARCHAR(256)] - -registration_date - [DATETIME] - -registration_hash - [VARCHAR(256)] - -username - [VARCHAR(512)] - NOT NULL + +ab_register_user + +id + [INTEGER] + NOT NULL + +email + [VARCHAR(512)] + NOT NULL + +first_name + [VARCHAR(256)] + NOT NULL + +last_name + [VARCHAR(256)] + NOT NULL + +password + [VARCHAR(256)] + +registration_date + [DATETIME] + +registration_hash + [VARCHAR(256)] + +username + [VARCHAR(512)] + NOT NULL ab_user - -ab_user - -id - [INTEGER] - NOT NULL - -active - [BOOLEAN] - -changed_by_fk - [INTEGER] - -changed_on - [DATETIME] - -created_by_fk - [INTEGER] - -created_on - [DATETIME] - -email - [VARCHAR(512)] - NOT NULL - -fail_login_count - [INTEGER] - -first_name - [VARCHAR(256)] - NOT NULL - -last_login - [DATETIME] - -last_name - [VARCHAR(256)] - NOT NULL - -login_count - [INTEGER] - -password - [VARCHAR(256)] - -username - [VARCHAR(512)] - NOT NULL + +ab_user + +id + [INTEGER] + NOT NULL + +active + [BOOLEAN] + +changed_by_fk + [INTEGER] + +changed_on + [DATETIME] + +created_by_fk + [INTEGER] + +created_on + [DATETIME] + +email + [VARCHAR(512)] + NOT NULL + +fail_login_count + [INTEGER] + +first_name + [VARCHAR(256)] + NOT NULL + +last_login + [DATETIME] + +last_name + [VARCHAR(256)] + NOT NULL + +login_count + [INTEGER] + +password + [VARCHAR(256)] + +username + [VARCHAR(512)] + NOT NULL ab_user--ab_user_role - -0..N -{0,1} + +0..N +{0,1} ab_user--ab_user - -0..N -{0,1} + +0..N +{0,1} ab_user--ab_user - -0..N -{0,1} - - - -dag_run_note - -dag_run_note - -dag_run_id - [INTEGER] - NOT NULL - -content - [VARCHAR(1000)] - -created_at - [TIMESTAMP] - NOT NULL - -updated_at - [TIMESTAMP] - NOT NULL - -user_id - [INTEGER] - - - -ab_user--dag_run_note - -0..N -{0,1} - - - -task_instance_note - -task_instance_note - -dag_id - [VARCHAR(250)] - NOT NULL - -map_index - [INTEGER] - NOT NULL - -run_id - [VARCHAR(250)] - NOT NULL - -task_id - [VARCHAR(250)] - NOT NULL - -content - [VARCHAR(1000)] - -created_at - [TIMESTAMP] - NOT NULL - -updated_at - [TIMESTAMP] - NOT NULL - -user_id - [INTEGER] - - - -ab_user--task_instance_note - -0..N -{0,1} + +0..N +{0,1} - + alembic_version - -alembic_version - -version_num - [VARCHAR(32)] - NOT NULL + +alembic_version + +version_num + [VARCHAR(32)] + NOT NULL - + callback_request - -callback_request - -id - [INTEGER] - NOT NULL - -callback_data - [JSON] - NOT NULL - -callback_type - [VARCHAR(20)] - NOT NULL - -created_at - [TIMESTAMP] - NOT NULL - -priority_weight - [INTEGER] - NOT NULL - -processor_subdir - [VARCHAR(2000)] + +callback_request + +id + [INTEGER] + NOT NULL + +callback_data + [JSON] + NOT NULL + +callback_type + [VARCHAR(20)] + NOT NULL + +created_at + [TIMESTAMP] + NOT NULL + +priority_weight + [INTEGER] + NOT NULL + +processor_subdir + [VARCHAR(2000)] - + connection - -connection - -id - [INTEGER] - NOT NULL - -conn_id - [VARCHAR(250)] - NOT NULL - -conn_type - [VARCHAR(500)] - NOT NULL - -description - [VARCHAR(5000)] - -extra - [TEXT] - -host - [VARCHAR(500)] - -is_encrypted - [BOOLEAN] - -is_extra_encrypted - [BOOLEAN] - -login - [VARCHAR(500)] - -password - [VARCHAR(5000)] - -port - [INTEGER] - -schema - [VARCHAR(500)] + +connection + +id + [INTEGER] + NOT NULL + +conn_id + [VARCHAR(250)] + NOT NULL + +conn_type + [VARCHAR(500)] + NOT NULL + +description + [VARCHAR(5000)] + +extra + [TEXT] + +host + [VARCHAR(500)] + +is_encrypted + [BOOLEAN] + +is_extra_encrypted + [BOOLEAN] + +login + [VARCHAR(500)] + +password + [VARCHAR(5000)] + +port + [INTEGER] + +schema + [VARCHAR(500)] - + dag - -dag - -dag_id - [VARCHAR(250)] - NOT NULL - -default_view - [VARCHAR(25)] - -description - [TEXT] - -fileloc - [VARCHAR(2000)] - -has_import_errors - [BOOLEAN] - -has_task_concurrency_limits - [BOOLEAN] - NOT NULL - -is_active - [BOOLEAN] - -is_paused - [BOOLEAN] - -is_subdag - [BOOLEAN] - -last_expired - [TIMESTAMP] - -last_parsed_time - [TIMESTAMP] - -last_pickled - [TIMESTAMP] - -max_active_runs - [INTEGER] - -max_active_tasks - [INTEGER] - NOT NULL - -next_dagrun - [TIMESTAMP] - -next_dagrun_create_after - [TIMESTAMP] - -next_dagrun_data_interval_end - [TIMESTAMP] - -next_dagrun_data_interval_start - [TIMESTAMP] - -owners - [VARCHAR(2000)] - -pickle_id - [INTEGER] - -processor_subdir - [VARCHAR(2000)] - -root_dag_id - [VARCHAR(250)] - -schedule_interval - [TEXT] - -scheduler_lock - [BOOLEAN] - -timetable_description - [VARCHAR(1000)] + +dag + +dag_id + [VARCHAR(250)] + NOT NULL + +default_view + [VARCHAR(25)] + +description + [TEXT] + +fileloc + [VARCHAR(2000)] + +has_import_errors + [BOOLEAN] + +has_task_concurrency_limits + [BOOLEAN] + NOT NULL + +is_active + [BOOLEAN] + +is_paused + [BOOLEAN] + +is_subdag + [BOOLEAN] + +last_expired + [TIMESTAMP] + +last_parsed_time + [TIMESTAMP] + +last_pickled + [TIMESTAMP] + +max_active_runs + [INTEGER] + +max_active_tasks + [INTEGER] + NOT NULL + +next_dagrun + [TIMESTAMP] + +next_dagrun_create_after + [TIMESTAMP] + +next_dagrun_data_interval_end + [TIMESTAMP] + +next_dagrun_data_interval_start + [TIMESTAMP] + +owners + [VARCHAR(2000)] + +pickle_id + [INTEGER] + +processor_subdir + [VARCHAR(2000)] + +root_dag_id + [VARCHAR(250)] + +schedule_interval + [TEXT] + +scheduler_lock + [BOOLEAN] + +timetable_description + [VARCHAR(1000)] - + dag_owner_attributes - -dag_owner_attributes - -dag_id - [VARCHAR(250)] - NOT NULL - -owner - [VARCHAR(500)] - NOT NULL - -link - [VARCHAR(500)] - NOT NULL + +dag_owner_attributes + +dag_id + [VARCHAR(250)] + NOT NULL + +owner + [VARCHAR(500)] + NOT NULL + +link + [VARCHAR(500)] + NOT NULL - + dag--dag_owner_attributes - -1 -1 + +1 +1 - + dag_schedule_dataset_reference - -dag_schedule_dataset_reference - -dag_id - [VARCHAR(250)] - NOT NULL - -dataset_id - [INTEGER] - NOT NULL - -created_at - [TIMESTAMP] - NOT NULL - -updated_at - [TIMESTAMP] - NOT NULL + +dag_schedule_dataset_reference + +dag_id + [VARCHAR(250)] + NOT NULL + +dataset_id + [INTEGER] + NOT NULL + +created_at + [TIMESTAMP] + NOT NULL + +updated_at + [TIMESTAMP] + NOT NULL - + dag--dag_schedule_dataset_reference - -1 -1 + +1 +1 - + dag_tag - -dag_tag - -dag_id - [VARCHAR(250)] - NOT NULL - -name - [VARCHAR(100)] - NOT NULL + +dag_tag + +dag_id + [VARCHAR(250)] + NOT NULL + +name + [VARCHAR(100)] + NOT NULL - + dag--dag_tag - -1 -1 + +1 +1 - + dag_warning - -dag_warning - -dag_id - [VARCHAR(250)] - NOT NULL - -warning_type - [VARCHAR(50)] - NOT NULL - -message - [TEXT] - NOT NULL - -timestamp - [TIMESTAMP] - NOT NULL + +dag_warning + +dag_id + [VARCHAR(250)] + NOT NULL + +warning_type + [VARCHAR(50)] + NOT NULL + +message + [TEXT] + NOT NULL + +timestamp + [TIMESTAMP] + NOT NULL - + dag--dag_warning - -1 -1 + +1 +1 - + dataset_dag_run_queue - -dataset_dag_run_queue - -dataset_id - [INTEGER] - NOT NULL - -target_dag_id - [VARCHAR(250)] - NOT NULL - -created_at - [TIMESTAMP] - NOT NULL + +dataset_dag_run_queue + +dataset_id + [INTEGER] + NOT NULL + +target_dag_id + [VARCHAR(250)] + NOT NULL + +created_at + [TIMESTAMP] + NOT NULL - + dag--dataset_dag_run_queue - -1 -1 + +1 +1 - + task_outlet_dataset_reference - -task_outlet_dataset_reference - -dag_id - [VARCHAR(250)] - NOT NULL - -dataset_id - [INTEGER] - NOT NULL - -task_id - [VARCHAR(250)] - NOT NULL - -created_at - [TIMESTAMP] - NOT NULL - -updated_at - [TIMESTAMP] - NOT NULL + +task_outlet_dataset_reference + +dag_id + [VARCHAR(250)] + NOT NULL + +dataset_id + [INTEGER] + NOT NULL + +task_id + [VARCHAR(250)] + NOT NULL + +created_at + [TIMESTAMP] + NOT NULL + +updated_at + [TIMESTAMP] + NOT NULL - + dag--task_outlet_dataset_reference - -1 -1 + +1 +1 - + dag_code - -dag_code - -fileloc_hash - [BIGINT] - NOT NULL - -fileloc - [VARCHAR(2000)] - NOT NULL - -last_updated - [TIMESTAMP] - NOT NULL - -source_code - [TEXT] - NOT NULL + +dag_code + +fileloc_hash + [BIGINT] + NOT NULL + +fileloc + [VARCHAR(2000)] + NOT NULL + +last_updated + [TIMESTAMP] + NOT NULL + +source_code + [TEXT] + NOT NULL - + dag_pickle - -dag_pickle - -id - [INTEGER] - NOT NULL - -created_dttm - [TIMESTAMP] - -pickle - [BLOB] - -pickle_hash - [BIGINT] + +dag_pickle + +id + [INTEGER] + NOT NULL + +created_dttm + [TIMESTAMP] + +pickle + [BLOB] + +pickle_hash + [BIGINT] - + dag_run - -dag_run - -id - [INTEGER] - NOT NULL - -clear_number - [INTEGER] - -conf - [BLOB] - -creating_job_id - [INTEGER] - -dag_hash - [VARCHAR(32)] - -dag_id - [VARCHAR(250)] - NOT NULL - -data_interval_end - [TIMESTAMP] - -data_interval_start - [TIMESTAMP] - -end_date - [TIMESTAMP] - -execution_date - [TIMESTAMP] - NOT NULL - -external_trigger - [BOOLEAN] - -last_scheduling_decision - [TIMESTAMP] - -log_template_id - [INTEGER] - -queued_at - [TIMESTAMP] - -run_id - [VARCHAR(250)] - NOT NULL - -run_type - [VARCHAR(50)] - NOT NULL - -start_date - [TIMESTAMP] - -state - [VARCHAR(50)] - -updated_at - [TIMESTAMP] + +dag_run + +id + [INTEGER] + NOT NULL + +clear_number + [INTEGER] + NOT NULL + +conf + [BLOB] + +creating_job_id + [INTEGER] + +dag_hash + [VARCHAR(32)] + +dag_id + [VARCHAR(250)] + NOT NULL + +data_interval_end + [TIMESTAMP] + +data_interval_start + [TIMESTAMP] + +end_date + [TIMESTAMP] + +execution_date + [TIMESTAMP] + NOT NULL + +external_trigger + [BOOLEAN] + +last_scheduling_decision + [TIMESTAMP] + +log_template_id + [INTEGER] + +queued_at + [TIMESTAMP] + +run_id + [VARCHAR(250)] + NOT NULL + +run_type + [VARCHAR(50)] + NOT NULL + +start_date + [TIMESTAMP] + +state + [VARCHAR(50)] + +updated_at + [TIMESTAMP] + + + +dag_run_note + +dag_run_note + +dag_run_id + [INTEGER] + NOT NULL + +content + [VARCHAR(1000)] + +created_at + [TIMESTAMP] + NOT NULL + +updated_at + [TIMESTAMP] + NOT NULL + +user_id + [INTEGER] - + dag_run--dag_run_note - -1 -1 + +1 +1 - + dagrun_dataset_event - -dagrun_dataset_event - -dag_run_id - [INTEGER] - NOT NULL - -event_id - [INTEGER] - NOT NULL + +dagrun_dataset_event + +dag_run_id + [INTEGER] + NOT NULL + +event_id + [INTEGER] + NOT NULL - + dag_run--dagrun_dataset_event - -1 -1 + +1 +1 - + task_instance - -task_instance - -dag_id - [VARCHAR(250)] - NOT NULL - -map_index - [INTEGER] - NOT NULL - -run_id - [VARCHAR(250)] - NOT NULL - -task_id - [VARCHAR(250)] - NOT NULL - -custom_operator_name - [VARCHAR(1000)] - -duration - [FLOAT] - -end_date - [TIMESTAMP] - -executor_config - [BLOB] - -external_executor_id - [VARCHAR(250)] - -hostname - [VARCHAR(1000)] - -job_id - [INTEGER] - -max_tries - [INTEGER] - -next_kwargs - [JSON] - -next_method - [VARCHAR(1000)] - -operator - [VARCHAR(1000)] - -pid - [INTEGER] - -pool - [VARCHAR(256)] - NOT NULL - -pool_slots - [INTEGER] - NOT NULL - -priority_weight - [INTEGER] - -queue - [VARCHAR(256)] - -queued_by_job_id - [INTEGER] - -queued_dttm - [TIMESTAMP] - -start_date - [TIMESTAMP] - -state - [VARCHAR(20)] - -trigger_id - [INTEGER] - -trigger_timeout - [DATETIME] - -try_number - [INTEGER] - -unixname - [VARCHAR(1000)] - -updated_at - [TIMESTAMP] + +task_instance + +dag_id + [VARCHAR(250)] + NOT NULL + +map_index + [INTEGER] + NOT NULL + +run_id + [VARCHAR(250)] + NOT NULL + +task_id + [VARCHAR(250)] + NOT NULL + +custom_operator_name + [VARCHAR(1000)] + +duration + [FLOAT] + +end_date + [TIMESTAMP] + +executor_config + [BLOB] + +external_executor_id + [VARCHAR(250)] + +hostname + [VARCHAR(1000)] + +job_id + [INTEGER] + +max_tries + [INTEGER] + +next_kwargs + [JSON] + +next_method + [VARCHAR(1000)] + +operator + [VARCHAR(1000)] + +pid + [INTEGER] + +pool + [VARCHAR(256)] + NOT NULL + +pool_slots + [INTEGER] + NOT NULL + +priority_weight + [INTEGER] + +queue + [VARCHAR(256)] + +queued_by_job_id + [INTEGER] + +queued_dttm + [TIMESTAMP] + +start_date + [TIMESTAMP] + +state + [VARCHAR(20)] + +trigger_id + [INTEGER] + +trigger_timeout + [DATETIME] + +try_number + [INTEGER] + +unixname + [VARCHAR(1000)] + +updated_at + [TIMESTAMP] - + dag_run--task_instance - -1 -1 + +1 +1 - + dag_run--task_instance - -1 -1 + +1 +1 - + task_reschedule - -task_reschedule - -id - [INTEGER] - NOT NULL - -dag_id - [VARCHAR(250)] - NOT NULL - -duration - [INTEGER] - NOT NULL - -end_date - [TIMESTAMP] - NOT NULL - -map_index - [INTEGER] - NOT NULL - -reschedule_date - [TIMESTAMP] - NOT NULL - -run_id - [VARCHAR(250)] - NOT NULL - -start_date - [TIMESTAMP] - NOT NULL - -task_id - [VARCHAR(250)] - NOT NULL - -try_number - [INTEGER] - NOT NULL + +task_reschedule + +id + [INTEGER] + NOT NULL + +dag_id + [VARCHAR(250)] + NOT NULL + +duration + [INTEGER] + NOT NULL + +end_date + [TIMESTAMP] + NOT NULL + +map_index + [INTEGER] + NOT NULL + +reschedule_date + [TIMESTAMP] + NOT NULL + +run_id + [VARCHAR(250)] + NOT NULL + +start_date + [TIMESTAMP] + NOT NULL + +task_id + [VARCHAR(250)] + NOT NULL + +try_number + [INTEGER] + NOT NULL - + dag_run--task_reschedule - -0..N -1 + +0..N +1 - + dag_run--task_reschedule - -0..N -1 - - - -task_instance--task_instance_note - -1 -1 - - - -task_instance--task_instance_note - -1 -1 - - - -task_instance--task_instance_note - -1 -1 - - - -task_instance--task_instance_note - -1 -1 + +0..N +1 - + task_instance--task_reschedule - -0..N -1 + +0..N +1 - + task_instance--task_reschedule - -0..N -1 + +0..N +1 - + task_instance--task_reschedule - -0..N -1 + +0..N +1 - + task_instance--task_reschedule - -0..N -1 + +0..N +1 - + rendered_task_instance_fields - -rendered_task_instance_fields - -dag_id - [VARCHAR(250)] - NOT NULL - -map_index - [INTEGER] - NOT NULL - -run_id - [VARCHAR(250)] - NOT NULL - -task_id - [VARCHAR(250)] - NOT NULL - -k8s_pod_yaml - [JSON] - -rendered_fields - [JSON] - NOT NULL + +rendered_task_instance_fields + +dag_id + [VARCHAR(250)] + NOT NULL + +map_index + [INTEGER] + NOT NULL + +run_id + [VARCHAR(250)] + NOT NULL + +task_id + [VARCHAR(250)] + NOT NULL + +k8s_pod_yaml + [JSON] + +rendered_fields + [JSON] + NOT NULL - + task_instance--rendered_task_instance_fields - -1 -1 + +1 +1 - + task_instance--rendered_task_instance_fields - -1 -1 + +1 +1 - + task_instance--rendered_task_instance_fields - -1 -1 + +1 +1 - + task_instance--rendered_task_instance_fields - -1 -1 + +1 +1 - + task_fail - -task_fail - -id - [INTEGER] - NOT NULL - -dag_id - [VARCHAR(250)] - NOT NULL - -duration - [INTEGER] - -end_date - [TIMESTAMP] - -map_index - [INTEGER] - NOT NULL - -run_id - [VARCHAR(250)] - NOT NULL - -start_date - [TIMESTAMP] - -task_id - [VARCHAR(250)] - NOT NULL + +task_fail + +id + [INTEGER] + NOT NULL + +dag_id + [VARCHAR(250)] + NOT NULL + +duration + [INTEGER] + +end_date + [TIMESTAMP] + +map_index + [INTEGER] + NOT NULL + +run_id + [VARCHAR(250)] + NOT NULL + +start_date + [TIMESTAMP] + +task_id + [VARCHAR(250)] + NOT NULL - + task_instance--task_fail - -0..N -1 + +0..N +1 - + task_instance--task_fail - -0..N -1 + +0..N +1 - + task_instance--task_fail - -0..N -1 + +0..N +1 - + task_instance--task_fail - -0..N -1 + +0..N +1 + + + +task_instance_note + +task_instance_note + +dag_id + [VARCHAR(250)] + NOT NULL + +map_index + [INTEGER] + NOT NULL + +run_id + [VARCHAR(250)] + NOT NULL + +task_id + [VARCHAR(250)] + NOT NULL + +content + [VARCHAR(1000)] + +created_at + [TIMESTAMP] + NOT NULL + +updated_at + [TIMESTAMP] + NOT NULL + +user_id + [INTEGER] + + + +task_instance--task_instance_note + +1 +1 + + + +task_instance--task_instance_note + +1 +1 + + + +task_instance--task_instance_note + +1 +1 + + + +task_instance--task_instance_note + +1 +1 task_map - -task_map - -dag_id - [VARCHAR(250)] - NOT NULL - -map_index - [INTEGER] - NOT NULL - -run_id - [VARCHAR(250)] - NOT NULL - -task_id - [VARCHAR(250)] - NOT NULL - -keys - [JSON] - -length - [INTEGER] - NOT NULL + +task_map + +dag_id + [VARCHAR(250)] + NOT NULL + +map_index + [INTEGER] + NOT NULL + +run_id + [VARCHAR(250)] + NOT NULL + +task_id + [VARCHAR(250)] + NOT NULL + +keys + [JSON] + +length + [INTEGER] + NOT NULL - + task_instance--task_map - -1 -1 + +1 +1 - + task_instance--task_map - -1 -1 + +1 +1 - + task_instance--task_map - -1 -1 + +1 +1 - + task_instance--task_map - -1 -1 + +1 +1 xcom - -xcom - -dag_run_id - [INTEGER] - NOT NULL - -key - [VARCHAR(512)] - NOT NULL - -map_index - [INTEGER] - NOT NULL - -task_id - [VARCHAR(250)] - NOT NULL - -dag_id - [VARCHAR(250)] - NOT NULL - -run_id - [VARCHAR(250)] - NOT NULL - -timestamp - [TIMESTAMP] - NOT NULL - -value - [BLOB] + +xcom + +dag_run_id + [INTEGER] + NOT NULL + +key + [VARCHAR(512)] + NOT NULL + +map_index + [INTEGER] + NOT NULL + +task_id + [VARCHAR(250)] + NOT NULL + +dag_id + [VARCHAR(250)] + NOT NULL + +run_id + [VARCHAR(250)] + NOT NULL + +timestamp + [TIMESTAMP] + NOT NULL + +value + [BLOB] - + task_instance--xcom - -0..N -1 + +0..N +1 - + task_instance--xcom - -1 -1 + +1 +1 - + task_instance--xcom - -0..N -1 + +0..N +1 - + task_instance--xcom - -1 -1 + +1 +1 - + log_template - -log_template - -id - [INTEGER] - NOT NULL - -created_at - [TIMESTAMP] - NOT NULL - -elasticsearch_id - [TEXT] - NOT NULL - -filename - [TEXT] - NOT NULL + +log_template + +id + [INTEGER] + NOT NULL + +created_at + [TIMESTAMP] + NOT NULL + +elasticsearch_id + [TEXT] + NOT NULL + +filename + [TEXT] + NOT NULL - + log_template--dag_run - -0..N -{0,1} + +0..N +{0,1} - + dataset - -dataset - -id - [INTEGER] - NOT NULL - -created_at - [TIMESTAMP] - NOT NULL - -extra - [JSON] - NOT NULL - -is_orphaned - [BOOLEAN] - NOT NULL - -updated_at - [TIMESTAMP] - NOT NULL - -uri - [VARCHAR(3000)] - NOT NULL + +dataset + +id + [INTEGER] + NOT NULL + +created_at + [TIMESTAMP] + NOT NULL + +extra + [JSON] + NOT NULL + +is_orphaned + [BOOLEAN] + NOT NULL + +updated_at + [TIMESTAMP] + NOT NULL + +uri + [VARCHAR(3000)] + NOT NULL - + dataset--dag_schedule_dataset_reference - -1 -1 + +1 +1 - + dataset--dataset_dag_run_queue - -1 -1 + +1 +1 - + dataset--task_outlet_dataset_reference - -1 -1 + +1 +1 - + dataset_event - -dataset_event - -id - [INTEGER] - NOT NULL - -dataset_id - [INTEGER] - NOT NULL - -extra - [JSON] - NOT NULL - -source_dag_id - [VARCHAR(250)] - -source_map_index - [INTEGER] - -source_run_id - [VARCHAR(250)] - -source_task_id - [VARCHAR(250)] - -timestamp - [TIMESTAMP] - NOT NULL + +dataset_event + +id + [INTEGER] + NOT NULL + +dataset_id + [INTEGER] + NOT NULL + +extra + [JSON] + NOT NULL + +source_dag_id + [VARCHAR(250)] + +source_map_index + [INTEGER] + +source_run_id + [VARCHAR(250)] + +source_task_id + [VARCHAR(250)] + +timestamp + [TIMESTAMP] + NOT NULL - + dataset_event--dagrun_dataset_event - -1 -1 + +1 +1 - + import_error - -import_error - -id - [INTEGER] - NOT NULL - -filename - [VARCHAR(1024)] - -stacktrace - [TEXT] - -timestamp - [TIMESTAMP] + +import_error + +id + [INTEGER] + NOT NULL + +filename + [VARCHAR(1024)] + +stacktrace + [TEXT] + +timestamp + [TIMESTAMP] - + job - -job - -id - [INTEGER] - NOT NULL - -dag_id - [VARCHAR(250)] - -end_date - [TIMESTAMP] - -executor_class - [VARCHAR(500)] - -hostname - [VARCHAR(500)] - -job_type - [VARCHAR(30)] - -latest_heartbeat - [TIMESTAMP] - -start_date - [TIMESTAMP] - -state - [VARCHAR(20)] - -unixname - [VARCHAR(1000)] + +job + +id + [INTEGER] + NOT NULL + +dag_id + [VARCHAR(250)] + +end_date + [TIMESTAMP] + +executor_class + [VARCHAR(500)] + +hostname + [VARCHAR(500)] + +job_type + [VARCHAR(30)] + +latest_heartbeat + [TIMESTAMP] + +start_date + [TIMESTAMP] + +state + [VARCHAR(20)] + +unixname + [VARCHAR(1000)] - + log - -log - -id - [INTEGER] - NOT NULL - -dag_id - [VARCHAR(250)] - -dttm - [TIMESTAMP] - -event - [VARCHAR(30)] - -execution_date - [TIMESTAMP] - -extra - [TEXT] - -map_index - [INTEGER] - -owner - [VARCHAR(500)] - -task_id - [VARCHAR(250)] + +log + +id + [INTEGER] + NOT NULL + +dag_id + [VARCHAR(250)] + +dttm + [TIMESTAMP] + +event + [VARCHAR(30)] + +execution_date + [TIMESTAMP] + +extra + [TEXT] + +friendly_owner + [VARCHAR(500)] + +map_index + [INTEGER] + +owner + [VARCHAR(500)] + +task_id + [VARCHAR(250)] trigger - -trigger - -id - [INTEGER] - NOT NULL - -classpath - [VARCHAR(1000)] - NOT NULL - -created_date - [TIMESTAMP] - NOT NULL - -kwargs - [JSON] - NOT NULL - -triggerer_id - [INTEGER] + +trigger + +id + [INTEGER] + NOT NULL + +classpath + [VARCHAR(1000)] + NOT NULL + +created_date + [TIMESTAMP] + NOT NULL + +kwargs + [JSON] + NOT NULL + +triggerer_id + [INTEGER] - + trigger--task_instance - -0..N -{0,1} + +0..N +{0,1} serialized_dag - -serialized_dag - -dag_id - [VARCHAR(250)] - NOT NULL - -dag_hash - [VARCHAR(32)] - NOT NULL - -data - [JSON] - -data_compressed - [BLOB] - -fileloc - [VARCHAR(2000)] - NOT NULL - -fileloc_hash - [BIGINT] - NOT NULL - -last_updated - [TIMESTAMP] - NOT NULL - -processor_subdir - [VARCHAR(2000)] + +serialized_dag + +dag_id + [VARCHAR(250)] + NOT NULL + +dag_hash + [VARCHAR(32)] + NOT NULL + +data + [JSON] + +data_compressed + [BLOB] + +fileloc + [VARCHAR(2000)] + NOT NULL + +fileloc_hash + [BIGINT] + NOT NULL + +last_updated + [TIMESTAMP] + NOT NULL + +processor_subdir + [VARCHAR(2000)] session - -session - -id - [INTEGER] - NOT NULL - -data - [BLOB] - -expiry - [DATETIME] - -session_id - [VARCHAR(255)] + +session + +id + [INTEGER] + NOT NULL + +data + [BLOB] + +expiry + [DATETIME] + +session_id + [VARCHAR(255)] sla_miss - -sla_miss - -dag_id - [VARCHAR(250)] - NOT NULL - -execution_date - [TIMESTAMP] - NOT NULL - -task_id - [VARCHAR(250)] - NOT NULL - -description - [TEXT] - -email_sent - [BOOLEAN] - -notification_sent - [BOOLEAN] - -timestamp - [TIMESTAMP] + +sla_miss + +dag_id + [VARCHAR(250)] + NOT NULL + +execution_date + [TIMESTAMP] + NOT NULL + +task_id + [VARCHAR(250)] + NOT NULL + +description + [TEXT] + +email_sent + [BOOLEAN] + +notification_sent + [BOOLEAN] + +timestamp + [TIMESTAMP] slot_pool - -slot_pool - -id - [INTEGER] - NOT NULL - -description - [TEXT] - -include_deferred - [BOOLEAN] - NOT NULL - -pool - [VARCHAR(256)] - -slots - [INTEGER] + +slot_pool + +id + [INTEGER] + NOT NULL + +description + [TEXT] + +include_deferred + [BOOLEAN] + NOT NULL + +pool + [VARCHAR(256)] + +slots + [INTEGER] variable - -variable - -id - [INTEGER] - NOT NULL - -description - [TEXT] - -is_encrypted - [BOOLEAN] - -key - [VARCHAR(250)] - -val - [TEXT] + +variable + +id + [INTEGER] + NOT NULL + +description + [TEXT] + +is_encrypted + [BOOLEAN] + +key + [VARCHAR(250)] + +val + [TEXT] diff --git a/docs/apache-airflow/migrations-ref.rst b/docs/apache-airflow/migrations-ref.rst index 8b0cc7248f5ae..cc39460fab99d 100644 --- a/docs/apache-airflow/migrations-ref.rst +++ b/docs/apache-airflow/migrations-ref.rst @@ -39,7 +39,9 @@ Here's the list of all the Database Migrations that are executed via when you ru +---------------------------------+-------------------+-------------------+--------------------------------------------------------------+ | Revision ID | Revises ID | Airflow Version | Description | +=================================+===================+===================+==============================================================+ -| ``375a816bbbf4`` (head) | ``405de8318b3a`` | ``2.8.0`` | add new field 'clear_number' to dagrun | +| ``f7bf2a57d0a6`` (head) | ``375a816bbbf4`` | ``2.8.0`` | Add friendly_owner to (Audit) Log table | ++---------------------------------+-------------------+-------------------+--------------------------------------------------------------+ +| ``375a816bbbf4`` | ``405de8318b3a`` | ``2.8.0`` | add new field 'clear_number' to dagrun | +---------------------------------+-------------------+-------------------+--------------------------------------------------------------+ | ``405de8318b3a`` | ``788397e78828`` | ``2.7.0`` | add include_deferred column to pool | +---------------------------------+-------------------+-------------------+--------------------------------------------------------------+ From 6fe978a68c063e18097934ae973bf49c4f55b47a Mon Sep 17 00:00:00 2001 From: Pat Garziglia <49124058+patgarz@users.noreply.github.com> Date: Tue, 5 Sep 2023 19:01:38 -0400 Subject: [PATCH 04/11] Apply code review suggestions to FAB Auth Manager --- airflow/auth/managers/fab/fab_auth_manager.py | 11 +++-------- tests/auth/managers/fab/test_fab_auth_manager.py | 1 + 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/airflow/auth/managers/fab/fab_auth_manager.py b/airflow/auth/managers/fab/fab_auth_manager.py index 4816da6b12270..55112a59db45f 100644 --- a/airflow/auth/managers/fab/fab_auth_manager.py +++ b/airflow/auth/managers/fab/fab_auth_manager.py @@ -65,8 +65,8 @@ def get_cli_commands() -> list[CLICommand]: def get_user_display_name(self) -> str: """Return the user's display name associated to the user in session.""" user = self.get_user() - first_name = user.first_name or "" - last_name = user.last_name or "" + first_name = user.first_name.strip() or "" + last_name = user.last_name.strip() or "" return f"{first_name} {last_name}".strip() def get_user_name(self) -> str: @@ -77,12 +77,7 @@ def get_user_name(self) -> str: email, or the database user ID. """ user = self.get_user() - if user.username: - return user.username - elif user.email: - return user.email - else: - return self.get_user_id() + return user.username or user.email or self.get_user_id() def get_user(self) -> User: """Return the user associated to the user in session.""" diff --git a/tests/auth/managers/fab/test_fab_auth_manager.py b/tests/auth/managers/fab/test_fab_auth_manager.py index 595b6f9db6e8b..eeb7a42e16f30 100644 --- a/tests/auth/managers/fab/test_fab_auth_manager.py +++ b/tests/auth/managers/fab/test_fab_auth_manager.py @@ -70,6 +70,7 @@ def test_get_user_name( (1, "First", None, "user", None, "First"), (1, None, "Last", "user", "email", "Last"), (1, None, None, None, "email", None), + (1, None, None, None, "email", False), ], ) @mock.patch.object(FabAuthManager, "get_user") From 7fd61e4008654387623eca93a8094384f05c15fe Mon Sep 17 00:00:00 2001 From: Pat Garziglia <49124058+patgarz@users.noreply.github.com> Date: Tue, 5 Sep 2023 19:05:37 -0400 Subject: [PATCH 05/11] Apply code review suggestions to Audit Log table changes --- ...0_add_friendly_owner_to_audit_log_table.py | 2 +- airflow/models/log.py | 8 ++--- airflow/www/decorators.py | 2 +- airflow/www/views.py | 6 ++-- docs/apache-airflow/img/airflow_erd.svg | 33 +++++++++++++++++++ 5 files changed, 42 insertions(+), 9 deletions(-) diff --git a/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py b/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py index 6a2b3ac79bc3d..93e06ce576a48 100644 --- a/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py +++ b/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py @@ -16,7 +16,7 @@ # specific language governing permissions and limitations # under the License. -"""Add friendly_owner to (Audit) Log table +"""Add owner_display_name to (Audit) Log table Revision ID: f7bf2a57d0a6 Revises: 375a816bbbf4 diff --git a/airflow/models/log.py b/airflow/models/log.py index adc4476417ea1..7431083d78c39 100644 --- a/airflow/models/log.py +++ b/airflow/models/log.py @@ -37,7 +37,7 @@ class Log(Base): event = Column(String(30)) execution_date = Column(UtcDateTime) owner = Column(String(500)) - friendly_owner = Column(String(500)) + owner_display_name = Column(String(500)) extra = Column(Text) __table_args__ = ( @@ -46,7 +46,7 @@ class Log(Base): Index("idx_log_event", event), ) - def __init__(self, event, task_instance=None, owner=None, friendly_owner=None, extra=None, **kwargs): + def __init__(self, event, task_instance=None, owner=None, owner_display_name=None, extra=None, **kwargs): self.dttm = timezone.utcnow() self.event = event self.extra = extra @@ -71,7 +71,7 @@ def __init__(self, event, task_instance=None, owner=None, friendly_owner=None, e self.map_index = kwargs["map_index"] self.owner = owner or task_owner - self.friendly_owner = friendly_owner or None + self.owner_display_name = owner_display_name or None def __str__(self) -> str: - return f"Log({self.event}, {self.task_id}, {self.owner}, {self.friendly_owner}, {self.extra})" + return f"Log({self.event}, {self.task_id}, {self.owner}, {self.owner_display_name}, {self.extra})" diff --git a/airflow/www/decorators.py b/airflow/www/decorators.py index 9469c562b36ca..26000f3a0a201 100644 --- a/airflow/www/decorators.py +++ b/airflow/www/decorators.py @@ -110,7 +110,7 @@ def wrapper(*args, **kwargs): event=event or f.__name__, task_instance=None, owner=user, - friendly_owner=user_display, + owner_display_name=user_display, extra=str(extra_fields), task_id=params.get("task_id"), dag_id=params.get("dag_id"), diff --git a/airflow/www/views.py b/airflow/www/views.py index 1e5dfa66f2c42..a0688421cd063 100644 --- a/airflow/www/views.py +++ b/airflow/www/views.py @@ -5503,7 +5503,7 @@ class LogModelView(AirflowModelView): "event", "execution_date", "owner", - "friendly_owner", + "owner_display_name", "extra", ] search_columns = [ @@ -5513,14 +5513,14 @@ class LogModelView(AirflowModelView): "event", "execution_date", "owner", - "friendly_owner", + "owner_display_name", "extra", ] label_columns = { "execution_date": "Logical Date", "owner": "Owner ID", - "friendly_owner": "Owner Name", + "owner_display_name": "Owner Name", } base_order = ("dttm", "desc") diff --git a/docs/apache-airflow/img/airflow_erd.svg b/docs/apache-airflow/img/airflow_erd.svg index 1aa4fb1b708c8..dc5f68ed3208c 100644 --- a/docs/apache-airflow/img/airflow_erd.svg +++ b/docs/apache-airflow/img/airflow_erd.svg @@ -1422,6 +1422,39 @@ log + +log + +id + [INTEGER] + NOT NULL + +dag_id + [VARCHAR(250)] + +dttm + [TIMESTAMP] + +event + [VARCHAR(30)] + +execution_date + [TIMESTAMP] + +extra + [TEXT] + +map_index + [INTEGER] + +owner + [VARCHAR(500)] + +owner_display_name + [VARCHAR(500)] + +task_id + [VARCHAR(250)] log From 428451d677c567676273f69171cec2382b1a9284 Mon Sep 17 00:00:00 2001 From: Pat Garziglia <49124058+patgarz@users.noreply.github.com> Date: Sat, 9 Sep 2023 12:12:59 -0400 Subject: [PATCH 06/11] Fix tests, type errors --- airflow/auth/managers/fab/fab_auth_manager.py | 4 ++-- airflow/www/decorators.py | 2 +- tests/auth/managers/fab/test_fab_auth_manager.py | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/airflow/auth/managers/fab/fab_auth_manager.py b/airflow/auth/managers/fab/fab_auth_manager.py index 55112a59db45f..290329dcc8530 100644 --- a/airflow/auth/managers/fab/fab_auth_manager.py +++ b/airflow/auth/managers/fab/fab_auth_manager.py @@ -65,8 +65,8 @@ def get_cli_commands() -> list[CLICommand]: def get_user_display_name(self) -> str: """Return the user's display name associated to the user in session.""" user = self.get_user() - first_name = user.first_name.strip() or "" - last_name = user.last_name.strip() or "" + first_name = user.first_name.strip() if isinstance(user.first_name, str) else "" + last_name = user.last_name.strip() if isinstance(user.last_name, str) else "" return f"{first_name} {last_name}".strip() def get_user_name(self) -> str: diff --git a/airflow/www/decorators.py b/airflow/www/decorators.py index 26000f3a0a201..db856f5de6d34 100644 --- a/airflow/www/decorators.py +++ b/airflow/www/decorators.py @@ -88,7 +88,7 @@ def wrapper(*args, **kwargs): with create_session() as session: if not get_auth_manager().is_logged_in(): user = "anonymous" - user_display = None + user_display = "" else: user = get_auth_manager().get_user_name() user_display = get_auth_manager().get_user_display_name() diff --git a/tests/auth/managers/fab/test_fab_auth_manager.py b/tests/auth/managers/fab/test_fab_auth_manager.py index eeb7a42e16f30..c245ec629cb0f 100644 --- a/tests/auth/managers/fab/test_fab_auth_manager.py +++ b/tests/auth/managers/fab/test_fab_auth_manager.py @@ -41,8 +41,8 @@ class TestFabAuthManager: @pytest.mark.parametrize( "id,first_name,last_name,username,email,expected", [ - (1, "First", "Last", None, None, 1), - (1, None, None, None, None, 1), + (1, "First", "Last", None, None, "1"), + (1, None, None, None, None, "1"), (1, "First", "Last", "user", None, "user"), (1, "First", "Last", "user", "email", "user"), (1, None, None, None, "email", "email"), @@ -69,8 +69,8 @@ def test_get_user_name( (1, "First", "Last", None, None, "First Last"), (1, "First", None, "user", None, "First"), (1, None, "Last", "user", "email", "Last"), - (1, None, None, None, "email", None), - (1, None, None, None, "email", False), + (1, None, None, None, "email", ""), + (1, None, None, None, "email", ""), ], ) @mock.patch.object(FabAuthManager, "get_user") From bd6edf4b9f9c2c59760fd1d3c57e4bed1deb0df9 Mon Sep 17 00:00:00 2001 From: Pat Garziglia <49124058+patgarz@users.noreply.github.com> Date: Tue, 12 Sep 2023 17:10:13 -0400 Subject: [PATCH 07/11] Add default Log schema value for owner_display_name --- .../0130_2_8_0_add_friendly_owner_to_audit_log_table.py | 2 +- docs/apache-airflow/img/airflow_erd.sha256 | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py b/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py index 93e06ce576a48..bdeed1ea8f09a 100644 --- a/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py +++ b/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py @@ -41,7 +41,7 @@ def upgrade(): """Adds owner_display_name column to log""" with op.batch_alter_table(TABLE_NAME) as batch_op: - batch_op.add_column(sa.Column("owner_display_name", sa.String(500))) + batch_op.add_column(sa.Column("owner_display_name", sa.String(500), server_default="")) def downgrade(): diff --git a/docs/apache-airflow/img/airflow_erd.sha256 b/docs/apache-airflow/img/airflow_erd.sha256 index cda5faa6048b1..8db9e03c41b81 100644 --- a/docs/apache-airflow/img/airflow_erd.sha256 +++ b/docs/apache-airflow/img/airflow_erd.sha256 @@ -1 +1 @@ -1c67d53d5e407f0f78745764892ec7ff3dc02fb60fa89c4fda32129f369b97a4 \ No newline at end of file +1c67d53d5e407f0f78745764892ec7ff3dc02fb60fa89c4fda32129f369b97a4 From 81ae0199409bdaad457b00d88e4adcef0c99d29a Mon Sep 17 00:00:00 2001 From: Pat Garziglia <49124058+patgarz@users.noreply.github.com> Date: Wed, 13 Sep 2023 00:29:32 -0400 Subject: [PATCH 08/11] Fix errors caused in rebase --- docs/apache-airflow/img/airflow_erd.sha256 | 2 +- docs/apache-airflow/img/airflow_erd.svg | 97 +++++++--------------- docs/apache-airflow/migrations-ref.rst | 2 +- 3 files changed, 34 insertions(+), 67 deletions(-) diff --git a/docs/apache-airflow/img/airflow_erd.sha256 b/docs/apache-airflow/img/airflow_erd.sha256 index 8db9e03c41b81..30071adffd63f 100644 --- a/docs/apache-airflow/img/airflow_erd.sha256 +++ b/docs/apache-airflow/img/airflow_erd.sha256 @@ -1 +1 @@ -1c67d53d5e407f0f78745764892ec7ff3dc02fb60fa89c4fda32129f369b97a4 +1c47d0bf6024dfaa76a5ef71ab1f98d97391dab46adb1d7a3cd35f406fc663a0 \ No newline at end of file diff --git a/docs/apache-airflow/img/airflow_erd.svg b/docs/apache-airflow/img/airflow_erd.svg index dc5f68ed3208c..197423aabe772 100644 --- a/docs/apache-airflow/img/airflow_erd.svg +++ b/docs/apache-airflow/img/airflow_erd.svg @@ -1422,72 +1422,39 @@ log - -log - -id - [INTEGER] - NOT NULL - -dag_id - [VARCHAR(250)] - -dttm - [TIMESTAMP] - -event - [VARCHAR(30)] - -execution_date - [TIMESTAMP] - -extra - [TEXT] - -map_index - [INTEGER] - -owner - [VARCHAR(500)] - -owner_display_name - [VARCHAR(500)] - -task_id - [VARCHAR(250)] - + log - -id - [INTEGER] - NOT NULL - -dag_id - [VARCHAR(250)] - -dttm - [TIMESTAMP] - -event - [VARCHAR(30)] - -execution_date - [TIMESTAMP] - -extra - [TEXT] - -friendly_owner - [VARCHAR(500)] - -map_index - [INTEGER] - -owner - [VARCHAR(500)] - -task_id - [VARCHAR(250)] + +id + [INTEGER] + NOT NULL + +dag_id + [VARCHAR(250)] + +dttm + [TIMESTAMP] + +event + [VARCHAR(30)] + +execution_date + [TIMESTAMP] + +extra + [TEXT] + +map_index + [INTEGER] + +owner + [VARCHAR(500)] + +owner_display_name + [VARCHAR(500)] + +task_id + [VARCHAR(250)] diff --git a/docs/apache-airflow/migrations-ref.rst b/docs/apache-airflow/migrations-ref.rst index cc39460fab99d..f0b2b121f5b16 100644 --- a/docs/apache-airflow/migrations-ref.rst +++ b/docs/apache-airflow/migrations-ref.rst @@ -39,7 +39,7 @@ Here's the list of all the Database Migrations that are executed via when you ru +---------------------------------+-------------------+-------------------+--------------------------------------------------------------+ | Revision ID | Revises ID | Airflow Version | Description | +=================================+===================+===================+==============================================================+ -| ``f7bf2a57d0a6`` (head) | ``375a816bbbf4`` | ``2.8.0`` | Add friendly_owner to (Audit) Log table | +| ``f7bf2a57d0a6`` (head) | ``375a816bbbf4`` | ``2.8.0`` | Add owner_display_name to (Audit) Log table | +---------------------------------+-------------------+-------------------+--------------------------------------------------------------+ | ``375a816bbbf4`` | ``405de8318b3a`` | ``2.8.0`` | add new field 'clear_number' to dagrun | +---------------------------------+-------------------+-------------------+--------------------------------------------------------------+ From d6c6d51d4ae0a9a5432f2fde0df19af6c6ced29d Mon Sep 17 00:00:00 2001 From: Pat Garziglia <49124058+patgarz@users.noreply.github.com> Date: Wed, 13 Sep 2023 01:16:50 -0400 Subject: [PATCH 09/11] Fix MSSQL default value column drop --- .../0130_2_8_0_add_friendly_owner_to_audit_log_table.py | 2 +- docs/apache-airflow/img/airflow_erd.sha256 | 2 +- docs/apache-airflow/img/airflow_erd.svg | 8 ++++---- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py b/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py index bdeed1ea8f09a..d161ea7f4118e 100644 --- a/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py +++ b/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py @@ -47,4 +47,4 @@ def upgrade(): def downgrade(): """Removes owner_display_name column from log""" with op.batch_alter_table(TABLE_NAME) as batch_op: - batch_op.drop_column("owner_display_name") + batch_op.drop_column("owner_display_name", mssql_drop_default=True) diff --git a/docs/apache-airflow/img/airflow_erd.sha256 b/docs/apache-airflow/img/airflow_erd.sha256 index 30071adffd63f..fde198f42e5b6 100644 --- a/docs/apache-airflow/img/airflow_erd.sha256 +++ b/docs/apache-airflow/img/airflow_erd.sha256 @@ -1 +1 @@ -1c47d0bf6024dfaa76a5ef71ab1f98d97391dab46adb1d7a3cd35f406fc663a0 \ No newline at end of file +be4a2570a16fc2def6da3bcf7f9072b26e6d9054f74bc91a105c396001cd042b \ No newline at end of file diff --git a/docs/apache-airflow/img/airflow_erd.svg b/docs/apache-airflow/img/airflow_erd.svg index 197423aabe772..61fa89b5b222f 100644 --- a/docs/apache-airflow/img/airflow_erd.svg +++ b/docs/apache-airflow/img/airflow_erd.svg @@ -1218,28 +1218,28 @@ task_instance--xcom -0..N +1 1 task_instance--xcom -1 +0..N 1 task_instance--xcom -0..N +1 1 task_instance--xcom -1 +0..N 1 From 53e87c7499eac92eda233df191cb1789a4c2e073 Mon Sep 17 00:00:00 2001 From: Pat Garziglia <49124058+patgarz@users.noreply.github.com> Date: Thu, 14 Sep 2023 18:47:04 -0400 Subject: [PATCH 10/11] Clarity: Change migration filename to match action --- ...30_2_8_0_add_owner_display_name_to_audit_log_table.py} | 0 docs/apache-airflow/img/airflow_erd.sha256 | 2 +- docs/apache-airflow/img/airflow_erd.svg | 8 ++++---- 3 files changed, 5 insertions(+), 5 deletions(-) rename airflow/migrations/versions/{0130_2_8_0_add_friendly_owner_to_audit_log_table.py => 0130_2_8_0_add_owner_display_name_to_audit_log_table.py} (100%) diff --git a/airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py b/airflow/migrations/versions/0130_2_8_0_add_owner_display_name_to_audit_log_table.py similarity index 100% rename from airflow/migrations/versions/0130_2_8_0_add_friendly_owner_to_audit_log_table.py rename to airflow/migrations/versions/0130_2_8_0_add_owner_display_name_to_audit_log_table.py diff --git a/docs/apache-airflow/img/airflow_erd.sha256 b/docs/apache-airflow/img/airflow_erd.sha256 index fde198f42e5b6..77ee7766776ff 100644 --- a/docs/apache-airflow/img/airflow_erd.sha256 +++ b/docs/apache-airflow/img/airflow_erd.sha256 @@ -1 +1 @@ -be4a2570a16fc2def6da3bcf7f9072b26e6d9054f74bc91a105c396001cd042b \ No newline at end of file +0f3980ba44d34afb93729f5e3206d78664532e8fbdef339449880fbdcbf35bb5 \ No newline at end of file diff --git a/docs/apache-airflow/img/airflow_erd.svg b/docs/apache-airflow/img/airflow_erd.svg index 61fa89b5b222f..197423aabe772 100644 --- a/docs/apache-airflow/img/airflow_erd.svg +++ b/docs/apache-airflow/img/airflow_erd.svg @@ -1218,28 +1218,28 @@ task_instance--xcom -1 +0..N 1 task_instance--xcom -0..N +1 1 task_instance--xcom -1 +0..N 1 task_instance--xcom -0..N +1 1 From c0b343e209c5d1a41788acf5d18b4cbb084a475b Mon Sep 17 00:00:00 2001 From: Pat Garziglia <49124058+patgarz@users.noreply.github.com> Date: Thu, 14 Sep 2023 21:27:29 -0400 Subject: [PATCH 11/11] Revert changes to default values --- ...130_2_8_0_add_owner_display_name_to_audit_log_table.py | 4 ++-- docs/apache-airflow/img/airflow_erd.sha256 | 2 +- docs/apache-airflow/img/airflow_erd.svg | 8 ++++---- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/airflow/migrations/versions/0130_2_8_0_add_owner_display_name_to_audit_log_table.py b/airflow/migrations/versions/0130_2_8_0_add_owner_display_name_to_audit_log_table.py index d161ea7f4118e..93e06ce576a48 100644 --- a/airflow/migrations/versions/0130_2_8_0_add_owner_display_name_to_audit_log_table.py +++ b/airflow/migrations/versions/0130_2_8_0_add_owner_display_name_to_audit_log_table.py @@ -41,10 +41,10 @@ def upgrade(): """Adds owner_display_name column to log""" with op.batch_alter_table(TABLE_NAME) as batch_op: - batch_op.add_column(sa.Column("owner_display_name", sa.String(500), server_default="")) + batch_op.add_column(sa.Column("owner_display_name", sa.String(500))) def downgrade(): """Removes owner_display_name column from log""" with op.batch_alter_table(TABLE_NAME) as batch_op: - batch_op.drop_column("owner_display_name", mssql_drop_default=True) + batch_op.drop_column("owner_display_name") diff --git a/docs/apache-airflow/img/airflow_erd.sha256 b/docs/apache-airflow/img/airflow_erd.sha256 index 77ee7766776ff..91deb334c310e 100644 --- a/docs/apache-airflow/img/airflow_erd.sha256 +++ b/docs/apache-airflow/img/airflow_erd.sha256 @@ -1 +1 @@ -0f3980ba44d34afb93729f5e3206d78664532e8fbdef339449880fbdcbf35bb5 \ No newline at end of file +ce9310390ebd98102b1c8193b1157e18c7d2871e6ca8df863e58a6a377b7d0a5 \ No newline at end of file diff --git a/docs/apache-airflow/img/airflow_erd.svg b/docs/apache-airflow/img/airflow_erd.svg index 197423aabe772..61fa89b5b222f 100644 --- a/docs/apache-airflow/img/airflow_erd.svg +++ b/docs/apache-airflow/img/airflow_erd.svg @@ -1218,28 +1218,28 @@ task_instance--xcom -0..N +1 1 task_instance--xcom -1 +0..N 1 task_instance--xcom -0..N +1 1 task_instance--xcom -1 +0..N 1