Skip to content

Add worker maintenance mode functionality to Edge3 provider UI #55301

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Add worker maintenance mode functionality to Edge3 provider UI #55301

wants to merge 5 commits into from
+463 −26

Conversation

dheerajturaga
Copy link
Contributor

  • Add POST /edge_worker/ui/worker/{worker_name}/maintenance endpoint to request maintenance mode
  • Add DELETE /edge_worker/ui/worker/{worker_name}/maintenance endpoint to exit maintenance mode
  • Add maintenance request form with required comment field (max 1024 chars)
  • Add maintenance state handling for all maintenance-related worker states
  • Add proper error handling and user feedback for maintenance operations
  • Add worker existence validation and database session management
  • Remove authentication dependencies for UI maintenance endpoints
  • Update frontend to show maintenance controls based on worker state
  • Format maintenance comments with timestamps for audit trail
Start Maintan image image

@boring-cyborg boring-cyborg bot added area:providers provider:edge Edge Executor / Worker (AIP-69) labels Sep 5, 2025
@dheerajturaga
Add worker maintenance mode functionality to Edge3 provider UI
39024c2 
  - Add POST /edge_worker/ui/worker/{worker_name}/maintenance endpoint to request maintenance mode
  - Add DELETE /edge_worker/ui/worker/{worker_name}/maintenance endpoint to exit maintenance mode
  - Add maintenance request form with required comment field (max 1024 chars)
  - Add maintenance state handling for all maintenance-related worker states
  - Add proper error handling and user feedback for maintenance operations
  - Add worker existence validation and database session management
  - Remove authentication dependencies for UI maintenance endpoints
  - Update frontend to show maintenance controls based on worker state
  - Format maintenance comments with timestamps for audit trail
@dheerajturaga dheerajturaga force-pushed the feature/edge-maintenance-plugin branch from 4fc6780 to 39024c2 Compare September 5, 2025 18:51
jscheffl
jscheffl reviewed Sep 5, 2025
View reviewed changes
Copy link
Contributor

@jscheffl jscheffl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very cool! I also wanted to get started with the maintenance but you opened the PR faster. Cool!
Functionally the skeleton is working.

Sorry about the many comments, don't be afraid. If too much then let me know and I can also contribute to this as this is an important feature urgently needed in UI for Airflow 3.1 release.

providers/edge3/src/airflow/providers/edge3/worker_api/routes/ui.py

@ui_router.post(
"/worker/{worker_name}/maintenance",
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To ensure this API is not "public/open" but authenticated, please add the access check

Suggested change
)
dependencies=[
Depends(requires_access_view(access_view=AccessView.JOBS)),
],
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jscheffl , I already tried this and I was not able to get the post and delete to work. I keep getting 401 not authorized and I was not able to figure out how to provide proper authorization.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good. But still needed. Probably because you implemented the manual authentication in the service accessor.
Can you attempt it via the axios generated code? If then not working, please push and I can help fixing.

providers/edge3/src/airflow/providers/edge3/worker_api/routes/ui.py

@ui_router.delete(
"/worker/{worker_name}/maintenance",
)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above

Suggested change
)
dependencies=[
Depends(requires_access_view(access_view=AccessView.JOBS)),
],
)

providers/edge3/src/airflow/providers/edge3/plugins/www/src/pages/WorkerPage.tsx Outdated
}
return (
<Button size="sm" colorScheme="blue" onClick={() => setActiveMaintenanceForm(workerName)}>
Enter Maintenance
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of (like in the 2.x legacy UI) rendering buttons with text - have you considered we use icons like in other tables/lists in the core UI?
Like here, icons on the right side:
image

For maintenance the icon https://react-icons.github.io/react-icons/search/#q=HiOutlineWrenchScrewdriver might be suitable (using this already for the state display)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To exit maintenance e.g. the icon HiOutlineArrowRightStartOnRectangle or vsc/VscDebugRestart could be suitable

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated to use the icons but I was not able to get tooltips working. It seems causing a Minified React Error 130

providers/edge3/src/airflow/providers/edge3/plugins/www/src/pages/WorkerPage.tsx Outdated
};

return (
<VStack gap={2} align="stretch">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As with react we have much more cool widgets and options compared to legacy 2.x UI, how about making this a modal dialog?
Can also be extracted as a component into a separate tsx file to be used as <MaintenanceForm worker={ worker } />

providers/edge3/src/airflow/providers/edge3/plugins/www/src/pages/WorkerPage.tsx Outdated
<Box fontSize="sm" whiteSpace="pre-wrap">
{worker.maintenance_comments || "No comment"}
</Box>
<Button size="sm" colorScheme="blue" onClick={() => exitMaintenance(workerName)}>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we make this UI form now "right" I assume we should add some kind of confirmation dialog/modal such that the user needs to confirm via a "are you sure you want to exit maintenance for worker XYZ? (yes/no)"

providers/edge3/src/airflow/providers/edge3/plugins/www/src/pages/WorkerPage.tsx Outdated
state === "maintenance pending" ||
state === "maintenance mode" ||
state === "maintenance request" ||
state === "maintenance exit" ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think when maintenance exit is already there as status we do not need to present the "Exit" button again, it is just pending that the worker fetches this.

Suggested change
state === "maintenance exit" ||

providers/edge3/src/airflow/providers/edge3/plugins/www/src/pages/WorkerPage.tsx Outdated

return (
<VStack gap={2} align="stretch">
<Textarea
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would be a cool improvement over 2.x UI to be able to use Markdown rendering here? WDYT? We might need to clone the component from core-ui. Might be also a beautification as a follow-up PR.

@dheerajturaga
Move MaintenanceRequest to datamodels_ui.py for better code organization
9a3ba47 
Remove explicit session.commit() calls - let SQLAlchemy handle transactions
@dheerajturaga
refactor: extract OperationsCell into separate component
106b0e6 
  - Move MaintenanceRequest class to datamodels_ui.py for better organization
  - Remove explicit session.commit() calls, let SQLAlchemy handle transactions
  - Extract OperationsCell and MaintenanceForm into separate component file
  - Clean up unused imports in WorkerPage.tsx
  - Improve code modularity and maintainability
@dheerajturaga
refactor: replace manual fetch with generated axios wrappers for main…
6943d1f 
…tenance operations

  - Use useUiServiceRequestWorkerMaintenance and useUiServiceExitWorkerMaintenance hooks
  - Remove manual fetch implementation with CSRF token handling
  - Simplify error handling using React Query's onSuccess/onError callbacks
  - Improve type safety with generated API client
  - Reduce bundle size and code complexity
  - Maintain same functionality with cleaner, more maintainable code
@dheerajturaga
Use react icons instead of old school buttons
b9015bc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jscheffl jscheffl jscheffl left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
area:providers provider:edge Edge Executor / Worker (AIP-69)
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dheerajturaga @jscheffl