ti-chi-bot[bot]
commented
2 months ago I have already done a preliminary review for you, and I hope to help you do a better job.
The pull request is a chore task, which changes the retry logic in the CI pipeline for TiFlash project. Specifically, it only retries when git fetch fails to update due to network, after getting the source code cache from ks3. This change aims to reduce the waiting time for getting the source code cache.
However, there are a few potential issues that need to be addressed. Firstly, there is no explicit reason why the change is necessary. Secondly, the retry logic is not configurable. It would be better to make the number of retries a parameter, so that it can be easily adjusted in the future. Finally, there is no information on how to test the changes.
To address these issues, the following suggestions can be made:
- Add a description of why this change is necessary. For example, is there any issue with the current retry logic that needs to be fixed?
- Make the number of retries a parameter and add a default value. This will make it easier to adjust in the future, without having to modify the code.
- Add instructions on how to test the changes. This can be done by adding a section in the pull request description or by adding a separate file with test instructions.
Overall, the changes seem reasonable, but the potential issues need to be addressed before merging.
Retry only after get src code cache from ks3. It takes about 7 minutes to retrieve and decompress the code cache from ks3. We only need to retry when git fetch fails to update due to network.