@Théo Zimmermann I am going to deploy the latest things from the bench check PR to see how they do
wait a minute
I am really uncomfortable with the use of exceptions in this code.
OK, what should I do instead?
I've just posted the review comments that I was writing.
FTR, the reason why I'm uncomfortable with exceptions in this situation is that with delayed computations (when using Lwt) I don't have a clear idea of where they will be raised and if they will really be caught by the exception handling mechanism that you put in place (I have strong doubts about that). So I really prefer handling errors with the result monad and keep exceptions for things that are not delayed and things that should not be caught (except by the outer exception handling mechanism).
@Théo Zimmermann I've refactored through the monad this time
@Théo Zimmermann Shall we deploy for a bit?
Yes, please go ahead.
it seems on push that the bench check isn't immediately created
I guess once the base jobs from gitlab come in it will begin to show
I don't know if we can improve this
other than that LGTM @Théo Zimmermann shall we merge upstream?
I'm fine with merging the check run PR if we are happy with the current behavior. Maybe give it a day or two to get more feedback.
I'll work on the reporting a bit today then. I had a look at the best place the trigger the report, I think whilst handling the check status might be a good option. What do you think?
I absolutely agree that this is where it should be handled.
In fact, when the bench is successful, getting the artifact for the table that you want and then pushing that to the check_run summary should be quite straightforward.
And in case the bench is unsuccessful, it would also be convenient to show some lines from the trace (like we usually do).
Hmm it seems that once the bench has succeeded. Starting a new bench doesn't update the checks
That seems to be because get_pull_request_id_and_milestone doesn't work without a milestone :(
OK, sorry for having suggested using it then.
Unfortunately, I probably won't have time to help today.
Last updated: May 28 2023 at 18:29 UTC