ferruzzi commented on PR #68106: URL: https://github.com/apache/airflow/pull/68106#issuecomment-4664600702
> Most of the test_triggerer_job.py tests fail and they stem from TriggerRunnerSupervisor not having a client available at construction time. The original code used cached_property to build the client lazily, but attrs overwrites cached_property during class setup, so the client was never actually built and callers received None. The fix approach I took to solving it was putting client initialization into start() before the object is constructed, matching the way that ActivitySubprocess already uses: > > ``` > pythonapi = in_process_api_server() > client = Client(base_url=None, token="", dry_run=True, transport=api.transport) > client.base_url = "http://in-process.invalid./"; > proc = super().start(..., client=client, ...) > ``` > > Tests that construct TriggerRunnerSupervisor directly now use client=mocker.Mock(spec=Client), and the lazy init test was removed. > > This fix is technically outside the scope of the _handle_request refactor in #65827. Should this be split into a separate PR, or is it fine here given it's a prerequisite for the tests to pass? > > Otherwise all test cases pass from: task-sdk and test_triggerer_job.py and test_processor.py > > @ferruzzi Yeah, moving client creation to start() look right and it's fine to include it in this PR since it's a prerequisite. But keep make_client() as a classmethod that start() calls rather than inlining it. It is there as an intentional override hook for subclasses. -- 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]
