JH-A-Kim commented on issue #65827: URL: https://github.com/apache/airflow/issues/65827#issuecomment-4685412097
Hi @ferruzzi, I had some review feedback on the PR that I raised and I wanted to get some confirmation before I do anything else. @ashb pointed out that the original elif isinstance chains in each supervisor were a deliberate opt-in design, each supervisor explicitly declares only the message types it needs to handle. The concern with the decorator registry he raised, is that it flips this to an opt-out structure. A new handler registered in request_handlers.py becomes available to all supervisors that call super(), including ones that may not need or should not handle that message type. This also means the benefit stated in the issue "no touching dispatch logic in any supervisor when adding a new shared message type", may be unsafe in cases where not every supervisor should handle that type. ashb suggested match/case, which would preserve the explicit opt-in contract per supervisor while cleaning up the syntax. The shared handler functions from request_handlers.py would still be used, so there's no implementation duplication just no registry on top. Just wanted to confirm how I should proceed! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
