RFR: 1680: NotifyBot::getPeriodicItems takes too long

[Magnus Ihse Bursie]

On Wed, 23 Nov 2022 18:53:27 GMT, Erik Joelsson <erikj at openjdk.org> wrote:

>> forge/src/main/java/org/openjdk/skara/forge/github/GitHubRepository.java line 300:
>> 
>>> 298:                 .onError(r -> r.statusCode() == 404 ? Optional.of(JSON.object().put("NOT_FOUND", true)) : Optional.empty())
>>> 299:                 .execute();
>>> 300:         if (branch.contains("NOT_FOUND")) {
>> 
>> I'm trying to figure out the logic here. So for a 404, you return a special "NOT_FOUND" object, which you detect after the request is done, to return Optional.empty().
>> 
>> But for any other errors..? This `var` syntax might be convenient when working in an IDE, but it sure makes code reviewing difficult. I assume `branch` is a JSON object, and that the results from onError are unwrapped before returning it from the request. But how will this work with the return statement below? 
>> 
>> branch.get("commit").get("sha") seems like it will fail with an exception...
>
> This is an established pattern in Skara that I just copied. The `onError` lambda returns an optional. If that optional is empty, then the default error behavior happens, which is to throw exception. So by only returning an object on 404, then that's the only result code we are changing the behavior on.
> 
> I'm not super satisfied with returning a JSON object with a special string in to signal something, it feels very scripty to me, but it's what we have.

Aha, I see. A bit odd design, but it works I guess.

-------------

PR: https://git.openjdk.org/skara/pull/1422