Hi again!

I have implemented the pagination and it works correctly now. It took me some time and eventually I have figured out that we don't need any special procedure to find minimal/maximum (timestamp, id) pair for builds in guile code since we already receive builds in the proper ordering from the database. So, we can extract first and last of the current page entries and that's it. :)

Besides that, I fixed some of the issues covered in the last review which I missed before. I am looking forward to merging the branches. Let me know if there is still something that needs to be fixed.

Best regards,
Tatiana

вс, 22 июл. 2018 г. в 16:57, Tatiana Sholokhova <tanja201396@gmail.com>:
Hi Clement,

Thank you for your advice! I resolved the 'with-critical-section' issue. To do so, I had to remove 'with-critical-section' call from 'handle-build-request' function and wrap critical section around each call of this function.
 
Today I am trying to fix the pagination and I will let you know about my results.

P. S.
Now I send this changes as separate small commits, we can rebase the history again when I finish with pagination.

Best regards,
Tatiana


сб, 21 июл. 2018 г. в 16:50, Clément Lassieur <clement@lassieur.org>:
Hello Tatiana!

Tatiana Sholokhova <tanja201396@gmail.com> writes:

> Hello Clément!
>
> Thank you for your review!
>
> I fixed most of the problems you noticed and rebased commits as you advised
> .
>
> I couldn't fix the problem with several calling of (with-critical-section).
> As I wrote to IRC channel, I tried to put '(with-critical-section
> db-channel (db)' around '(let* ...)' and I received an error:
> ```
>    In web/server.scm:
>    279:25  0 (_)
> Throw to key `vm-error' with args `(vm-run "Too few values returned to
> continuation" ())'.
> ```

It's because 'respond-html' returns several values.  I think you could
do:

(respond-html
  (with-critical-section ...
    (let* ...)))

> Could you give a status about the pagination?
>>
> Pagination works correctly with evaluations, but it doesn't work correctly
> with builds. In some cases, we have builds missing. It happens due to equal
> timestamp values, so we need to filter build by (timestamp, id) tuple key.
>
> What else do we need to do before the merge?

Once we have something consistent, we can push.  And we can add stuff
afterwards of course.  Do you think it would be feasible to fix the
pagination before the merge?

I won't have time to look at your update before tomorrow night, I'll let
you know then!

Thanks,
Clément