imrichardwu commented on PR #61878: URL: https://github.com/apache/airflow/pull/61878#issuecomment-3930754665
> I think this needs some refactoring. The current implementation looks functional but it is not very clear. I think the algorithm should be as follows: > > 1. If `max_length <= 0`, `return "" ` > 2. Build the truncation message once and, if `max_length `is smaller than its length, return it immediately. > 3. Determine the quoting strategy and compute the formatting overhead (prefix + quotes + suffix) and calculate available space. > 4. If available space is less than `MIN_CONTENT_LENGTH`, return the truncation message only. > 5. Otherwise slice the content to fit and construct the final string and ensure the result does not exceed `max_length`, trimming it if necessary. > > Currently, I think you are mixing 3, 4 and 5 when they should each be in disparate blocks of code. And this is resulting in needless duplication. > > Also, I was just wondering why this has to be duplicated across 2 files? Perhaps, a maintainer/committer could weigh in on this but is it possible to have this in `airflow.utils.helpers` instead? I implemented all your feedback and would love for you to review my pr again! -- 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]
