jrderuiter commented on PR #41639: URL: https://github.com/apache/airflow/pull/41639#issuecomment-2330817101
This duplication is essentially following the existing pattern that's already in place in the hook (if you look at the other methods, there is very often both a sync and an async implementation). To reduce duplication we could break out the setup (lines 430-434) and the checking of the token (lines 450-456) into separate functions but I'm not sure if that would really improve readability. A more heavy-weight alternative would be to only define the async version and then define the sync version using [asyncio.run](https://docs.python.org/3/library/asyncio-runner.html#asyncio.run): ``` def _get_aad_token_for_default_az_credential(self, resource: str) -> str: return asyncio.run(self. _a_get_aad_token_for_default_az_credential(resource) ``` However, this would add some overhead to the sync function call as `asyncio.run` starts an async event loop in the background: > This function runs the passed coroutine, taking care of managing the asyncio event loop, finalizing asynchronous generators, and closing the executor. In short, to match the existing style I would prefer leaving it as is. However, I'm happy to explore these other options if needed. Also open to any suggestions from others :) -- 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]
