Fix model loading order of DagRunNote vs User

[Kache]

It seems DagRun gets loaded before User, resulting in a
sqlalchemy.exc.NoReferencedTableError (at least when running airflow tasks test DAG_ID TASK_ID) when defining DagRun's foreign key to the
still nonexistent User table.

Referencing the column object instead of using str establishes proper
import ordering and fixes the issue.

Fix: #34109

[potiuk]

I don't think this is a good approach. We are trying to move out FAB to separate provider (see #32210 and likely we do not want to keep reference to Fab user table in Note. That would be something to be solved - I think - in the way to break the relation altogether - this field in Note should accept any user id - not only one coming from FAB user table.

[potiuk]

cc: @vincbeck

[vincbeck]

I don't think this is a good approach. We are trying to move out FAB to separate provider (see #32210 and likely we do not want to keep reference to Fab user table in Note. That would be something to be solved - I think - in the way to break the relation altogether - this field in Note should accept any user id - not only one coming from FAB user table.

Good catch! And I +1 on this one :)

[vincbeck]

As mentioned, we dont want to reference the User table in core Airflow

[Kache]

Thanks for elaborating on the broader implications.

Isn't "trying to move out FAB to separate provider" a separate feature/refactor concern? It's simply currently the case that User is referenced in DagRunNote as "ab_user.id", and this change is just making the current state of things explicit enough for Python to load modules in dependency order.

This change shouldn't make moving out FAB more difficult either. Would you guys think it sufficient to add TODO comments in reference to #32210 or additional notes elsewhere?

Alternatively, perhaps the foreign key constraint could be removed entirely, but that seems to require a migration which requires a bit more ceremony that I'm not familiar with.

[vincbeck]

This change shouldn't make moving out FAB more difficult either. Would you guys think it sufficient to add TODO comments in reference to #32210 or additional notes elsewhere?

I am not in favor of this one. This is postponing the problem and does not solve it. I am not too familiar either but removing the foreign key might be the solution for that. I'd wait for @potiuk to confirm though

[potiuk]

Isn't "trying to move out FAB to separate provider" a separate feature/refactor concern? It's simply currently the case that User is referenced in DagRunNote as "ab_user.id", and this change is just making the current state of things explicit enough for Python to load modules in dependency order.

This change shouldn't make moving out FAB more difficult either. Would you guys think it sufficient to add TODO comments in reference to #32210 or additional notes elsewhere?

I believe it's the same issue and there is 0 chance this PR will be out in 2.7.4 (if we have one) - it will at most get out in 2.8.0 (tentatively a month from now) - by which time we definitely want to have FAB as separate provider, and even if we do not make it on time, we should at least break the links between core DB and FAB DB.

So I'd rather treat this one as "thing to rememeber to solve WHEN we move". Merging it now when we know it's going to be changed later makes little sense and adds no value.

[potiuk]

I am not in favor of this one. This is postponing the problem and does not solve it. I am not too familiar either but removing the foreign key might be the solution for that. I'd wait for @potiuk to confirm though

Yeah - as explained above - we should (separate PR) - remove the Foreign Key (and add migration to do so) - even if we will get the Provider separation later. This will solve the problem as well, and will be step in the direction we are heading rather than orthogonal to it.

[Kache]

I see, thanks for explaining. The workaround ought to suffice in the meantime, anyways.