Misleading terminology in UI: "Active" or "Unpaused"

[XD-DENG]

(This may be too minor. If anyone thinks it's not necessary to make any change, I can definitely understand as well.)

This thought started with the discussion @ashb and I had at #14306 (comment) . So the definition of active in an Airflow DAG context is "Whether that DAG was seen on the last DagBag load".

However, on the other hand, in some scenarios of Airflow, active is used equivalently to UN-paused, like the home page.

To avoid potential confusion in terminology usage, shall we consider changing Active in home page to something like unpaused (or a better word).

Again, it's minor to me, and would be happy to close this if most folks find it ok as it is now.

[XD-DENG]

@ashb @kaxil @turbaszek @ryanahamilton may you please let me know your thoughts on this? Thanks.

[ashb]

I remember Ryan and I talked about this when he added/restyled those toggles.

Active is almost entirely an internal concept, so perhaps the best thing to rename is that, and we can leave active for this use here (cos "All/Unpaused/Paused" just sounds really odd to me for some reason.)

[turbaszek]

I think splitting this into All and Unpaused would be enough but +1 to renaming Active to Unpaused to align naming.

[ryanahamilton]

As @ashb mentioned, there was some discussion around this when this filter was refactored in #8106. I'd prefer @ashb's suggestion of keeping the "active" designation for this purpose if possible. Using a negative prefix of "un" ("not paused") isn't great from a UX perspective of differentiating status labels.

[XD-DENG]

Thanks everyone for your inputs!

What I would propose is:

Let's change the "internal" term is_active to something else. It brings two benefits:

• to avoid the potential confusion
• better represent what it really means ("Whether that DAG was seen on the last DagBag load").

Candidates can be something like is_active -> is_present.

May you please:

• share if you think it's necessary to do this, or it's too minor and you think we can live with it.
• if we do this, do you think is_present is a good replacement? or any other suggestion?

Thanks!

[ashb]

👍 to your proposal.

[XD-DENG]

I'm preparing a PR for this issue (rename is_active into is_present in dag table).

I have encountering again the same issue that I met in #14581 (comment): migration 2c6edca13270, Resource based permissions is querying the DB using the column that we intend to rename, and this results in exception.

I'm not sure if any change should be made to 2c6edca13270, Resource based permissions. Especially given it's already rolled out in 2.0.0 and 2.0.1, it may not be straightforward to refactor this migration. But maybe someone can kindly make some good suggestions?

In #14581 (rename last_scheduler_run into last_parsed_time), we managed to fix the issue by limiting the columns needed, so the renamed column is not needed and we can work around the problem. But this time, the column we intend to rename IS needed (chain: 2c6edca13270, Resource based permissions -> create_app -> sync_appbuilder_roles -> sync_roles -> create_dag_specific_permissions, in which is_active is used in filter, and we now rename it into is_present).

If we want to use the same trick again to fix this problem, we need to refactor create_dag_specific_permissions. For example, I don't understand why create_dag_specific_permissions only applied to "all active and paused DAGs". Why not all DAGs? If it can be simplified to all DAGs, we can apply the same trick again to fix this migration issue.

@kaxil @ashb @turbaszek Kindly let me know your suggestions? Happy to clarify and have further discussion. Many thanks!

[ashb]

/cc @jhtimmins who wrote that original migration too

[XD-DENG]

Gentle ping @jhtimmins

[jhtimmins]

Thanks for the ping @XD-DENG.

I think that is_present is too ambiguous (present in what? why would this change?). in_last_dagbag_load may be overly verbose, but I'd lean towards that or something similar to minimize the context needed to understand the variable.

Regarding the filtering on "all active and paused DAGs" -- Looks like that logic was added in the original dag-specific permissions PR was added 3 years ago f3f2eb3. None of the design docs for this are accessible anymore, so I'm not sure what the reasoning was.

My guess is that this was included so that active DAGs that were deleted through the UI don't stick around causing no-longer-relevant permissions to get created.

[XD-DENG]

Thanks @jhtimmins .

Putting the naming or condition check questions aside, possibly what we may need more inputs from you is the migration script 2c6edca13270, Resource based permissions.

We don't have to make the change proposed in this Issue. But earlier or later, people may need to make some changes to the dag table (e.g. renaming a column like what I'm doing here), then they will face the issue I have encountered.

@kaxil please correct me if any point is incorrect (as per our discussion at #14581 (comment)) or in case you have any other things to add. Thanks.

[XD-DENG]

Regarding why we would like to rename is_active into something else like is_present, please refer to our discussions above in this Issue. Feel free to propose if you have any idea. Cheers.

[kaxil]

@kaxil please correct me if any point is incorrect (as per our discussion at #14581 (comment)) or in case you have any other things to add. Thanks.

Yup that is correct, if we rename or remove the DagModel it will break the migrations. Historically we have created a Dummy Class instead of importing one from Airflow to avoid such situation, example:

airflow/airflow/migrations/versions/8646922c8a04_change_default_pool_slots_to_1.py

Lines 48 to 72 in 4372d45

	class TaskInstance(Base): # noqa: D101 # type: ignore
	__tablename__ = "task_instance"
	task_id = Column(String(ID_LEN, **COLLATION_ARGS), primary_key=True)
	dag_id = Column(String(ID_LEN, **COLLATION_ARGS), primary_key=True)
	execution_date = Column(UtcDateTime, primary_key=True)
	start_date = Column(UtcDateTime)
	end_date = Column(UtcDateTime)
	duration = Column(Float)
	state = Column(String(20))
	_try_number = Column('try_number', Integer, default=0)
	max_tries = Column(Integer)
	hostname = Column(String(1000))
	unixname = Column(String(1000))
	job_id = Column(Integer)
	pool = Column(String(50), nullable=False)
	pool_slots = Column(Integer, default=1)
	queue = Column(String(256))
	priority_weight = Column(Integer)
	operator = Column(String(1000))
	queued_dttm = Column(UtcDateTime)
	queued_by_job_id = Column(Integer)
	pid = Column(Integer)
	executor_config = Column(PickleType(pickler=dill))
	external_executor_id = Column(String(ID_LEN, **COLLATION_ARGS))

[manuzhang]

I don't thinks this is minor now when I'm trying to get DAGs from this tab from the /dags endpoint with only_active parameter. Meanwhile, is there a way to get all unpaused dags?

[kaxil]

I don't thinks this is minor now when I'm trying to get DAGs from this tab from the /dags endpoint with only_active parameter. Meanwhile, is there a way to get all unpaused dags?

I don't think so .. cc @ephraimbuddy -- We should probably have a "unpaused" dags filter in that endpoint too and explain the difference of "unpaused" vs "active" in the spec and docs.

[uranusjr]

Just a minor point, it’s probably easier to just change the is_active field on the model, without changing the actual column name at the db level, i.e.

is_present = Column("is_active", Boolean, default=False)

[potiuk]

I just tried to describe the current status in #22025 based on confused user discussion #21864 . And there are far too many "yes this is confusing" there.

I have another proposal how to solve it. MINOR and "super-simple.

How about we simply rename "Active" to "Unpaused" in the UI? This is, what it is, there is no particular reason to name the tab "Active" because it's meaning is "Unpaused" really. We could come up with another name as well "Schedulable" ?

[potiuk]

Anyone else who would like to have a say here ?

I think tthis is really more of a "UX" thing. @bbovenzi - what do you think - maybe really just renamiing the "Active" to "Unpaused" in the UI would solve the problem?