Changing dag_id to a positional argument in the 'dags list-runs' CLI command

[dirrao]

Changing dag_id to a positional argument in the 'dags list-runs' CLI command

[dstandish]

Can anyone comment on why we are doing this?

[vikramkoka]

@dirrao Why did we make a breaking change here?

[dirrao]

@dirrao Why did we make a breaking change here?

@vikramkoka This breaking change has been intentionally planned for Airflow 3.0.

[ashb]

@dirrao why? Was there a mailing list discussion about it?

In future please include the "why" and the context in the PR descriptions. Right now we are in the dark as to the reason for this change!

Reviwers @potiuk @o-nikolas @vincbeck: Please can we be better about enforcing what we say we want out of PRs: https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst

• Adhere to guidelines for commit messages described in this article. This makes the lives of those who come after you (and your future self) a lot easier.

[potiuk]

Reviwers @potiuk @o-nikolas @vincbeck: Please can we be better about enforcing what we say we want out of PRs:

Sure, I am all for it. and usually do. But when you review 1000 PRs, sometimes one or two slip through. When you review almost none, it's far less easy to make such mistake for sure.

Things like that happen especially when you review many PRs and I hope we can get a fair share of those reviewed by everyone.

If that is a consistent problem rather than one time problem, it's indeed good to address it. But (unless there is some data about it) I think we are good in pointing out whys to be perfectly honest.

@ashb do you think we have a consistent problem with it (what?) and that we should do some change rather than pointing three people in single PR that exhibited that behaviour?

[potiuk]

@dstandish
Can anyone comment on why we are doing this?
@vikramkoka
@dirrao Why did we make a breaking change here?

@vikramkoka @dirrao I made a very quick check now and quick git blame on that #TODO shows it was added by @jedcunningham here #26357 explaining why the #TODO was there and mentions previous attempt of doing it #25978 - and I also suggest to look for those things when you look for sources, because we are all humans and make mistakes and it only takes a very little (took me about 2 minutes). search through our repository if we accidentally missed that newsfragment here - without unnecesary raising the tone of the discussion and finger-pointing people. I think it's a good thing that we collaborate here, and cover each-others' back when we make mistake rather than point at them.

And yes, if it is still legitimate (@jedcunningham ?) It should be included in newsfragment (and this is what we all forgot to ask @dirrao to do here - althought in probably other 500 of those we did not).

I simply thing collaboration and gentle - possibly even humorous - way of pointing out mistakes rather than fingerpointing is a better way to build collaboration spirit.

[potiuk]

BTW. Pretty much always when I fix someone's mistake (including mine) I say PR #XYZ introduced the problem that I found and fixed. I pretty much never write this person here made a mistake. Highly recommend that approach as well

[dirrao]

@dirrao why? Was there a mailing list discussion about it?

In future please include the "why" and the context in the PR descriptions. Right now we are in the dark as to the reason for this change!

• Adhere to guidelines for commit messages described in this article. This makes the lives of those who come after you (and your future self) a lot easier.

Thank you for the feedback, and I completely understand the concern about the lack of context in the PR description.

To clarify, this change was made based on the comment in the code:
TODO: convert this to a positional arg in Airflow 3.

Moving forward, future PRs will include detailed explanations of the "why" and any relevant context. Please let me know if there’s anything I might have missed or any additional action required on my side.

Thank you for highlighting this.

[jedcunningham]

I think we should be critical of all "TODOs" etc that were tagged for Airflow 3. We should view them as ideas for potential changes, not hard commitments, especially when breaking changes are involved. We've been pretty lax at throwing "in AF3" around in the past (which is good imo, dream big).

In this specific case, I came in and reverted a breaking change that was already merged. Then I left a breadcrumb that we should consider doing the switch when we are doing breaking changes.

If I were writing it from scratch? Positional. Is it worth a breaking change for users? Maybe? But we've been more breaking-change-adverse than I imagined we would be 2 years ago, so maybe not.