Skip to content

Show task instance in web UI by custom operator name#31852

Merged
uranusjr merged 16 commits into
apache:mainfrom
utkarsharma2:operator_name
Jul 6, 2023
Merged

Show task instance in web UI by custom operator name#31852
uranusjr merged 16 commits into
apache:mainfrom
utkarsharma2:operator_name

Conversation

@utkarsharma2

@utkarsharma2 utkarsharma2 commented Jun 12, 2023

Copy link
Copy Markdown
Contributor

we should use operator_name when displaying the operator name in UI for taskinstance object.

Related: #28466

@boring-cyborg boring-cyborg Bot added area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues labels Jun 12, 2023
@utkarsharma2
utkarsharma2 marked this pull request as draft June 12, 2023 06:04
@utkarsharma2 utkarsharma2 changed the title [Webserver] Use operator_name instead of task_type in UI for task object [Webserver] Use operator_name instead of task_type in UI for taskinstance object Jun 12, 2023
@utkarsharma2 utkarsharma2 changed the title [Webserver] Use operator_name instead of task_type in UI for taskinstance object [Webserver] Use custom_operator_name instead of task_type in UI for taskinstance object Jun 12, 2023
@utkarsharma2
utkarsharma2 marked this pull request as ready for review June 13, 2023 00:20
Comment thread airflow/models/taskinstance.py Outdated
@utkarsharma2
utkarsharma2 requested a review from uranusjr June 13, 2023 13:41
Comment thread airflow/www/views.py Outdated

@pierrejeambrun pierrejeambrun Jun 14, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think to avoid this we can replace the @property by an sqlalchemy column_property. (Would also allow filtering, and querying against it easily)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pierrejeambrun I believe to do this we would do something like this

operator_name = column_property(
        select(
            func.IF(custom_operator_name is not None, custom_operator_name, operator)
        )
    )

But the IF statement is not working with sqlite3. Getting below error -
Screenshot 2023-06-15 at 10 00 57 AM

Is there any other way to accomplish this?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think CASE is better supported across databases

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give that a shot.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uranusjr @pierrejeambrun using column_property raises below issue
AttributeError: Neither 'Label' object nor 'Comparator' object has an attribute 'nullable' when looking for a solution ran into this thread - marshmallow-code/marshmallow-sqlalchemy#97

As mentioned in the thread it's fixed in 0.28.0 But we cannot upgrade to 0.28.0 because of the below issue -
flask-appbuilder 4.3.1 requires marshmallow-sqlalchemy<0.27.0,>=0.22.0,
flask-appbuilder is pinned to 4.3.1 here -

"flask-appbuilder[oauth]==4.3.1",

Unfortunately even in the latest version of flask-appbuilder 4.3.2 we still have upperbound on marshmallow-sqlalchemy>=0.22.0, <0.27.0
https://github.com/dpgaspar/Flask-AppBuilder/blob/9090d096f697cf03dd80ebc656a6b0e79a05adb3/setup.py#LL61

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The thread mentions adding an explicit field. Maybe this can be worked around by adding an operator_name field in the Marshmallow model?

marshmallow-code/marshmallow-sqlalchemy#97 (comment)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@uranusjr AFAIK, I don't think we are generating Marshmallow models directly, it's done flask_appbuilder/api/convert.py. Is there any other way that I'm not aware of?

@uranusjr

Copy link
Copy Markdown
Member

@pierrejeambrun Would it be OK if we go with the current code and investigate column_property later? That seems more complicated than anticipated.

Comment thread airflow/models/taskinstance.py Outdated
@utkarsharma2
utkarsharma2 requested a review from uranusjr July 4, 2023 11:20
@hussein-awala hussein-awala added this to the Airflow 2.7.0 milestone Jul 5, 2023
@uranusjr
uranusjr merged commit f6115a3 into apache:main Jul 6, 2023
@uranusjr uranusjr changed the title [Webserver] Use custom_operator_name instead of task_type in UI for taskinstance object Show task instance in web UI by custom operator name Jul 6, 2023
@ephraimbuddy ephraimbuddy added the type:new-feature Changelog: New Features label Jul 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:UI Related to UI/UX. For Frontend Developers. area:webserver Webserver related Issues type:new-feature Changelog: New Features

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants