unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Luciana Lima Brito <lubrito@posteo.net>
To: Christopher Baines <mail@cbaines.net>
Cc: guix-devel@gnu.org
Subject: Re: Outreachy - Guix Data Service: questions about improving the data for derivation comparisons.
Date: Sun, 25 Apr 2021 20:15:07 +0000	[thread overview]
Message-ID: <20210425171507.6a259017@lubrito> (raw)
In-Reply-To: <87bla4hdto.fsf@cbaines.net>

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

Hi

Your advices helped me think more clearly.

There was no need to create or modify structures other than what I was
already changing. I now return an alist instead of a list on the
derivation-differences-* functions on comparison.scm (for outputs,
inputs and sources). It helped to simplify the mapping on
controller.scm. The changes on html.scm were minimal, basically it is
matching on pairs, instead of single values.

Two questions:

1 - The match on the html expects 5 values for "outputs", so I had to
settle on using empty objects on the JSON, when needed, else it would
break the match on the html. Is it ok?

2 - Now on controller.scm "outputs", "inputs", "sources", and even
"arguments" have the same structure, which is an alist of the form:

  ((base . (...))
   (target . (...))
   (common . (...)))

and I'm using the same map and match-lambda code to process them,
wouldn't it be reasonable now to make it a local function?

I'm sending the patch. I'll be waiting your reviews.

-- 
Best Regards,

Luciana Lima Brito
MSc. in Computer Science

[-- Attachment #2: 0001-Change-handling-of-queried-data-for-derivations-comp.patch --]
[-- Type: text/x-patch, Size: 11946 bytes --]

From d605d519a684b1be57ebd09cdf697bcdba017da1 Mon Sep 17 00:00:00 2001
From: Luciana Brito <lubrito@posteo.net>
Date: Sun, 25 Apr 2021 15:17:33 -0300
Subject: [PATCH] Change handling of queried data for derivations comparison.

comparison.scm: return query data for derivation comparison as an alist, instead of list.
html.scm: match on pairs, instead of single values.
controller.scm: simplify mapping on outputs/inputs/sources.
---
 guix-data-service/comparison.scm             | 68 +++++++++++---------
 guix-data-service/web/compare/controller.scm | 62 ++++--------------
 guix-data-service/web/compare/html.scm       | 43 ++++++-------
 3 files changed, 69 insertions(+), 104 deletions(-)

diff --git a/guix-data-service/comparison.scm b/guix-data-service/comparison.scm
index e5e1955..1f47c38 100644
--- a/guix-data-service/comparison.scm
+++ b/guix-data-service/comparison.scm
@@ -158,19 +158,23 @@ GROUP BY 1, 2, 3, 4, 5"))
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-            (list output-name
-                  path
-                  hash-algorithm
-                  hash
-                  recursive
-                  (append (if (memq base-derivation-id
-                                    parsed-derivation-ids)
-                              '(base)
-                              '())
-                          (if (memq target-derivation-id
-                                    parsed-derivation-ids)
-                              '(target)
-                              '()))))))
+            `((output-name . ,output-name)
+              (path . ,path)
+              ,@(if (string? hash-algorithm)
+                    `((hash-algorithm . ,hash-algorithm))
+                    `((hash-algorithm . ())))
+              ,@(if (string? hash)
+                    `((hash . ,hash))
+                    `((hash . ())))
+              (recursive . ,(string=? recursive "t"))
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define (derivation-inputs-differences-data conn
@@ -202,16 +206,16 @@ INNER JOIN derivations ON derivation_outputs.derivation_id = derivations.id
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-          (list derivation_file_name
-                derivation_output_name
-                (append (if (memq base-derivation-id
-                                  parsed-derivation-ids)
-                            '(base)
-                            '())
-                        (if (memq target-derivation-id
-                                  parsed-derivation-ids)
-                            '(target)
-                            '()))))))
+            `((derivation_file_name . ,derivation_file_name)
+              (derivation_output_name . ,derivation_output_name)
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define (derivation-sources-differences-data conn
@@ -235,15 +239,15 @@ GROUP BY derivation_source_files.store_path"))
           (let ((parsed-derivation-ids
                  (map string->number
                       (parse-postgresql-array-string derivation_ids))))
-            (list store_path
-                  (append (if (memq base-derivation-id
-                                    parsed-derivation-ids)
-                              '(base)
-                              '())
-                          (if (memq target-derivation-id
-                                    parsed-derivation-ids)
-                              '(target)
-                              '()))))))
+            `((store_path . ,store_path)
+              ,(append (if (memq base-derivation-id
+                                 parsed-derivation-ids)
+                           '(base)
+                           '())
+                       (if (memq target-derivation-id
+                                 parsed-derivation-ids)
+                           '(target)
+                           '()))))))
        (exec-query conn query)))
 
 (define* (package-derivation-differences-data conn
diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-service/web/compare/controller.scm
index 895bb40..9ef8e5b 100644
--- a/guix-data-service/web/compare/controller.scm
+++ b/guix-data-service/web/compare/controller.scm
@@ -590,60 +590,24 @@
             ((application/json)
              (let ((outputs
                     (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((name path hash-alg hash recursive)
-                                  `((name . ,name)
-                                    (path . ,path)
-                                    ,@(if (string? hash-alg)
-                                          `((hash-algorithm . ,hash-alg))
-                                          '())
-                                    ,@(if (string? hash)
-                                          `((hash . ,hash))
-                                          '())
-                                    (recursive . ,(string=? recursive "t")))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((output-groups (assq-ref data 'outputs)))
-                       (list (assq-ref output-groups 'base)
-                             (assq-ref output-groups 'target)
-                             (assq-ref output-groups 'common)))))
+                     (match-lambda
+                       ((label values ...)
+                        `(,label . ,(list->vector values))))
+                     (assq-ref data 'outputs)))
 
                    (inputs
                     (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((derivation output)
-                                  `((derivation . ,derivation)
-                                    (output . ,output))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((input-groups (assq-ref data 'inputs)))
-                       (list (assq-ref input-groups 'base)
-                             (assq-ref input-groups 'target)
-                             (assq-ref input-groups 'common)))))
+                     (match-lambda
+                       ((label values ...)
+                        `(,label . ,(list->vector values))))
+                     (assq-ref data 'inputs)))
 
                    (sources
                     (map
-                     (lambda (label items)
-                       (cons label
-                             (list->vector
-                              (map
-                               (match-lambda
-                                 ((derivation)
-                                  `((derivation . ,derivation))))
-                               (or items '())))))
-                     '(base target common)
-                     (let ((source-groups (assq-ref data 'sources)))
-                       (list (assq-ref source-groups 'base)
-                             (assq-ref source-groups 'target)
-                             (assq-ref source-groups 'common)))))
+                     (match-lambda
+                       ((label values ...)
+                        `(,label . ,(list->vector values))))
+                     (assq-ref data 'sources)))
 
                    (arguments
                     (map
@@ -651,7 +615,7 @@
                        ((label args ...)
                         `(,label . ,(list->vector args))))
                      (assq-ref data 'arguments))))
-
+               
                (render-json
                 `((base                  . ((derivation . ,base-derivation)))
                   (target                . ((derivation . ,target-derivation)))
diff --git a/guix-data-service/web/compare/html.scm b/guix-data-service/web/compare/html.scm
index 5b5fe0a..30cc499 100644
--- a/guix-data-service/web/compare/html.scm
+++ b/guix-data-service/web/compare/html.scm
@@ -487,27 +487,24 @@
                  (th "Hash")
                  (th "Recursive")))
                (tbody
-                ,@(let ((base-outputs (assq-ref outputs 'base))
-                        (target-outputs (assq-ref outputs 'target))
-                        (common-outputs (assq-ref outputs 'common)))
-                    (append-map
-                     (lambda (label items)
-                       (map
-                        (match-lambda
-                          ((name path hash-algorithm hash recursive)
-                           `(tr
-                             (td ,label)
-                             (td ,name)
-                             (td (a (@ (href ,path))
-                                    ,(display-store-item path)))
-                             (td ,hash-algorithm)
-                             (td ,hash)
-                             (td ,recursive))))
-                        (or items '())))
-                     (list base target "Common")
-                     (list (assq-ref outputs 'base)
-                           (assq-ref outputs 'target)
-                           (assq-ref outputs 'common))))))))
+                ,@(append-map
+                   (lambda (label items)
+                     (map
+                      (match-lambda
+                        (((_ . name) (_ . path) (_ . hash-alg) (_ . hash) (_ . recursive))
+                         `(tr
+                           (td ,label)
+                           (td ,name)
+                           (td (a (@ (href ,path))
+                                  ,(display-store-item path)))
+                           (td ,hash-alg)
+                           (td ,hash)
+                           (td ,recursive))))
+                      (or items '())))
+                   (list base target "Common")
+                   (list (assq-ref outputs 'base)
+                         (assq-ref outputs 'target)
+                         (assq-ref outputs 'common)))))))
         (h2 "Inputs")
         ,@(let ((inputs (assq-ref data 'inputs)))
             `((table
@@ -522,7 +519,7 @@
                    (lambda (label items)
                      (map
                       (match-lambda
-                        ((derivation outputs)
+                        (((_ . derivation) (_ . outputs))
                          `(tr
                            (td ,label)
                            (td (a (@ (href ,derivation))
@@ -546,7 +543,7 @@
                    (lambda (label items)
                      (map
                       (match-lambda
-                        ((file)
+                        (((_ . file))
                          `(tr
                            (td ,label)
                            (td (a (@ (href ,file))
-- 
2.30.2


  reply	other threads:[~2021-04-25 20:17 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-21 18:29 Outreachy - Guix Data Service: questions about improving the data for derivation comparisons Luciana Lima Brito
2021-04-22  7:53 ` Christopher Baines
2021-04-22 20:00   ` Luciana Lima Brito
2021-04-22 20:08     ` Christopher Baines
2021-04-22 21:02       ` Luciana Lima Brito
2021-04-22 21:15         ` Christopher Baines
2021-04-23 21:15           ` Luciana Lima Brito
2021-04-23 21:48             ` Christopher Baines
2021-04-25 20:15               ` Luciana Lima Brito [this message]
2021-04-26  8:15                 ` Christopher Baines
2021-04-26 19:11                   ` Luciana Lima Brito
2021-04-26 21:21                     ` Christopher Baines
2021-04-27 13:10                       ` Luciana Lima Brito
2021-04-27 18:23                         ` Christopher Baines
2021-04-27 18:33                         ` Luciana Lima Brito
2021-04-27 18:42                           ` Christopher Baines
2021-04-27 19:53                             ` Luciana Lima Brito
2021-04-27 20:29                               ` Christopher Baines
2021-04-27 22:35                                 ` Luciana Lima Brito
2021-04-28  7:56                                   ` Christopher Baines

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210425171507.6a259017@lubrito \
    --to=lubrito@posteo.net \
    --cc=guix-devel@gnu.org \
    --cc=mail@cbaines.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).