You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by GitBox <gi...@apache.org> on 2022/07/07 05:15:34 UTC

[GitHub] [airflow] rino0601 commented on pull request #24810: Revert "Add %z for %(asctime)s to fix timezone for logs on UI (#24373)"

rino0601 commented on PR #24810:
URL: https://github.com/apache/airflow/pull/24810#issuecomment-1177082287

   @jedcunningham @bbovenzi 
   
   
   If only #24811 is merged, it will be a squash commit mixed with changes to `airflow/www/static/js/grid/details/content/taskInstance/Logs/utils.js` (here in after `Logs/utils.js` ).
   
   Because `v2-3-stable` branch doesn't have `Logs/utils.js` yet, 
   changing `Logs/utils.js` will cause a conflict when we `git cherry-pick` on `v2-3-test` later.
   
   To avoid conflict, there are 3 options IMHO
   
   **opt1. cherry-pick a dependency commit before cherry-pick #24811**
   the commit creating `Logs/utils.js` needs to be cherry-picked first. `Logs/utils.js` appears since #24404. It has several related PRs and looks still in progress to complete feature. If we only cherry-pick #24404 , it will contain an incomplete function and is not appropriate.
   On the other hand, if we cherry-pick all related commits first, it does not seem appropriate for patch release.
   
   **opt2. handle conflict when #24811 is cherry-picked**
   maintainer who cherry-picks into `v2-3-test` have to manually fix the conflict.
   which is need a lot of efforts.
   I think this is the reason why the previous PR(#24373) was omitted in 2.3.3
   
   **opt3. exclude changing `Logs/utils.js`**
   So I divided the PR into two. If a revert PR merged to main first, #24811 's sqaush commit don't change `Logs/utils.js` , it makes cherry-picking easier.
   There is also a way to make a PR that is easy to cherry-pick without reverting, but then useless regular expressions are left in `Logs/utils.js`. If left unattended, it will become a factor of potential bugs, so we will have to clean it by a separate PR. The only difference is whether we do it first or later to clean the bad codes that already merged, either way, we need two PRs.
   
   If #24403 and it's related can be released as 2.3.4, there is no need to split the PR. (can choose opt1)
   However, those PRs are unlabeled, but they are looks very new features... so they're likely going to 2.4.0. I think.
   
   I hope this has been explained, but if you still think that one PR is better, please close this PR. Then, I will write down what to do when a conflict occurs in the #24811. But my English is a mess, so I don't know if it can be explained.
   
   Or if I'm misunderstanding something, please let me know.


-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org