sunildataengineer commented on PR #68298: URL: https://github.com/apache/airflow/pull/68298#issuecomment-4931290248
@dabla @bugraoz93 Thank you for the thorough review feedback. I've been working through your suggestions and I want to acknowledge the journey here: What I learned: The async implementation needs to be truly async, not just sync methods wrapped in async_to_sync — you were right about the performance implications SFTPOperation should be an Enum for type safety and cleaner comparisons, not string constants The transfer logic has real duplication that deserves a DRY refactor, and the handler pattern you suggested makes the code much cleaner Even small details matter — like using self.execute_complete.__name__ instead of hardcoded strings What I struggled with: Balancing the async implementation — understanding how to properly use asyncio.Semaphore and asyncio.gather with native async SFTP operations The enum refactor — making sure the operation constants are used consistently across sync/async paths and in the trigger Making sure I actually pushed all the resolved changes (that "git push" reminder was a wake-up call!) Keeping track of which feedback was still unresolved vs. what I thought I'd already fixed I've now rebased onto main and pushed the latest branch with the fixes. I'm confident the changes address your feedback, but I know there's always more to learn from experienced reviewers like yourselves. Could you please take another look when you have a moment? Any remaining feedback is welcome — I'm here to get this right. Thanks for your patience and for pushing me to write better code. -- 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]
