RFR: 1806: Optimize the CSR progress message and log when CSR not found [v3]
Zhao Song
zsong at openjdk.org
Fri Jan 27 00:39:46 UTC 2023
On Fri, 27 Jan 2023 00:08:38 GMT, Zhao Song <zsong at openjdk.org> wrote:
>> It needs to be solved in some way. Catching `Exception` and ignoring it is not allowed in bot code. That could be triggered by an IOException, or failure in contacting the remote or almost anything else. If that happens, we need to handle the failure correct so that the WorkItem will be retried.
>>
>> BotUtils.getVersion either can't throw RuntimeException when it can't find a .jcheck/conf file, or we need to handle the fallback to the override repo. The latter would be cleaner, but we probably don't really need that support. I think returning Optional.empty() from BotUtils.getVersion() is probably ok if no .jcheck/conf file could be found.
>
>> It needs to be solved in some way. Catching `Exception` and ignoring it is not allowed in bot code. That could be triggered by an IOException, or failure in contacting the remote or almost anything else. If that happens, we need to handle the failure correct so that the WorkItem will be retried.
>>
>> BotUtils.getVersion either can't throw RuntimeException when it can't find a .jcheck/conf file, or we need to handle the fallback to the override repo. The latter would be cleaner, but we probably don't really need that support. I think returning Optional.empty() from BotUtils.getVersion() is probably ok if no .jcheck/conf file could be found.
>
> Got it! I will fix it
Now I realized that this jcheck conf issue happens because the place of `JdkVersion version = BotUtils.getVersion(pr).orElse(null);`. In `CheckWorkItem`, we have check for jcheck configuration in **target branch**. So if jcheck conf missing or invalid in **target branch**, the bot will return immediately.
However, in `BotUtils#getVersion`, the bot would try to get conf from **source branch** first. Although we also have check for **source branch**, it is after the call of JdkVersion version = BotUtils.getVersion(pr).orElse(null);`. So bad conf in **source branch** would cause getVersion throw exceptions.
-------------
PR: https://git.openjdk.org/skara/pull/1463
More information about the skara-dev
mailing list