Implement backfill banner #47411
Implement backfill banner #47411
Conversation
aritra24
requested review from
bbovenzi,
jscheffl,
pierrejeambrun and
ryanahamilton
as code owners
|
We only use this banner once. I think you can combined the two components, Banner and BackfillBanner, and not worry about arrays and keys. |
|
Hmm, do you foresee there being a use for it in any other place in the future? If so I'd leave it in. Else I'll go ahead with merging the two |
Let merge it for now and we can always split it apart in the future. |
|
Removed the usage of useEffect, it seems to work now. |
|
I think there's still some buggy behaviour so I'll leave it as wip, will check it out some more tomorrow 🤔 |
I see it's marked as ready for review. Did you figure out the buggy behavior? |
|
Hey @bbovenzi, this is still a wip. Might've been a miss from my side for it to be ready for review. There's still a bug i think where sometimes on reload the banner doesn't show up. I'm trying to nail down why that is. Might take me a couple days to get to it. |
|
Nice work. Thanks for being so patient through my reviews! |
|
Thanks for the detailed and insightful reviews, it's been a learning experience! |
* WIP: Implement backfill banner resolves: apache#43968 * Removed generic Banner in favor of BackfillBanner * Removed usage of useEffect * Added pause/unpause and stop backfill in the banner * Update backfill filter and some names * Removed visibility button * Resolved review comments
2 participants
resolves: #43968
^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named
{pr_number}.significant.rstor{issue_number}.significant.rst, in newsfragments.