-
Notifications
You must be signed in to change notification settings - Fork 16.3k
Fix Link to Dag in Plugin #55642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix Link to Dag in Plugin #55642
Conversation
df249ad to
c24bf0d
Compare
|
@jscheffl I just pushed a commit that should solve your problems Screen.Recording.2025-09-29.at.12.59.41.movI'm creating a PR for the 'bootstraping tool'. (pre-commit hook is not happy, but the error message seems unrelated to my change, let me know if you manage to fix it) |
|
Here is main PR: #56205 |
Cool! I was thinking about we probably also need to pseudo assume/build the bootstrapping tool and have dependabot running over it such that packages are upgraded regularly... still one of my items on my bcuket list... |
4940dce to
81e8f98
Compare
|
@pierrejeambrun thanks for the hints! I attempted to apply your proposals to Edge Executor UI Plugin and it seems to work.... but also needed to realize that https://github.com/apache/airflow/pull/55642/files#diff-72d1ca6b75418dccf2e70084d6d3b9840da702fa127e3848f7fd66198f9d406eR68 was missing on the PR to main. Unfortunately w/o this it is still not working with main. As proposed in https://apache-airflow.slack.com/archives/C0809U4S1Q9/p1759237779941759?thread_ts=1759181599.821269&cid=C0809U4S1Q9 I implemented a detection in "InternalLink" component which captures the diff. ...unfortunately I assume due to missing property in
Any idea how I can model a workaround keeping compatibility with 3.1.0 and later versions? UPDATE: After fiddeling and triaging for ~2 hours it seems this is a real breaking change, mainly caused by https://github.com/apache/airflow/pull/55642/files#diff-11195c94c2a05c1187b0517dfe1437a2914f4b22a5b2a9d721cb253ba16e4f79R41 - I can make it either working on 3.1.0 or 3.1.0++ but not on both because the vite.config.ts seems to be mutually exclusive? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed, I think it will be a substantial effort to make it work between new/old plugins version and <=3.1.1 >=3.1.1 core versions
I think we should leverage the experimental tag of the react app plugin feature, and add a lower bound on airflow core for the new edge provider version plugin. (have airflow >=3.1.1 in new edge provider version) if that's acceptable. That would save us some ugly backward compatibility code. (if we manage to make it work)
Yep. We can make such exception and our tooling support it, though, it's rather uncommon to have >= patchlevel. More likely >= 3.2.0 ? |
81e8f98 to
cb2448a
Compare
|
@pierrejeambrun @kaxil @eladkal FYI seems this is needed to make Edge UI Plugin working in 3.1.1 release. The previous releases (incl 1.4.0rc1) are "broken" as UI routes are having a breaking change... good that the tagged the React plugins "experimental" :-D |
providers/edge3/src/airflow/providers/edge3/plugins/edge_executor_plugin.py
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice thanks, just a couple of comments.
|
@jscheffl I texted this PR with base url sub path and a nginx redirect. Its working as expected. |
|
Thanks for testing as well!
Not working "at all" or just UI Plugin broken? I saw it working in general just w/o proper UI plugin support. |
You are right! I shall correct myself, egde executor and workers are working. The UI plugin is broken |
pierrejeambrun
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me, thanks Jens.

There is a teething problem that the React Router DOM "Link" component (similar like #55641) is not working in Edge React UI - pointers to outside the plugins do not work. I need some React Expertise to fix it seems.
@shubhamraj-git / @bbovenzi / @pierrejeambrun HELP :-D
To make this reproducible I made small modifications to revert my current workaround:
Switch back to the "right" internal link from "react-router-dom"
How to see the error:
breeze start-airflow --python 3.12 --backend postgres --executor EdgeExecutor --load-example-dagsIs this a limitation of the plugin concept or a bug in my Plugin? How can the Toaster be made working?
On main I use an "external link" with href from Chakra which leads to a full page realod which is nasty and slow.
Is there a way from within the Plugin to link to the Dag run/grid view w/o page reload?