From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mp2 ([2001:41d0:8:6d80::]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) by ms0.migadu.com with LMTPS id KEMQNKT9d2Dv6wAAgWs5BA (envelope-from ) for ; Thu, 15 Apr 2021 10:47:32 +0200 Received: from aspmx1.migadu.com ([2001:41d0:8:6d80::]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)) by mp2 with LMTPS id aGfFLqT9d2BDBgAAB5/wlQ (envelope-from ) for ; Thu, 15 Apr 2021 08:47:32 +0000 Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by aspmx1.migadu.com (Postfix) with ESMTPS id 4CFB1245CF for ; Thu, 15 Apr 2021 10:47:32 +0200 (CEST) Received: from localhost ([::1]:52326 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lWxet-0000r8-9V for larch@yhetil.org; Thu, 15 Apr 2021 04:47:31 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:49552) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lWxdn-0000pZ-Mo for guix-devel@gnu.org; Thu, 15 Apr 2021 04:46:23 -0400 Received: from mira.cbaines.net ([212.71.252.8]:40834) by eggs.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lWxdi-0000Fp-Q2 for guix-devel@gnu.org; Thu, 15 Apr 2021 04:46:23 -0400 Received: from localhost (unknown [IPv6:2a02:8010:68c1:0:8ac0:b4c7:f5c8:7caa]) by mira.cbaines.net (Postfix) with ESMTPSA id 7336C27BC76; Thu, 15 Apr 2021 09:46:15 +0100 (BST) Received: from capella (localhost [127.0.0.1]) by localhost (OpenSMTPD) with ESMTP id 2f42dc9a; Thu, 15 Apr 2021 08:46:14 +0000 (UTC) References: <20210414164859.7acc631f@lubrito> User-agent: mu4e 1.4.15; emacs 27.1 From: Christopher Baines To: Luciana Lima Brito Subject: Re: Outreachy - Guix Data Service: implementing basic json output for derivation comparison page In-reply-to: <20210414164859.7acc631f@lubrito> Date: Thu, 15 Apr 2021 09:46:12 +0100 Message-ID: <87wnt4x7e3.fsf@cbaines.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Received-SPF: pass client-ip=212.71.252.8; envelope-from=mail@cbaines.net; helo=mira.cbaines.net X-Spam_score_int: -18 X-Spam_score: -1.9 X-Spam_bar: - X-Spam_report: (-1.9 / 5.0 requ) BAYES_00=-1.9, SPF_HELO_PASS=-0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: guix-devel@gnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: "Development of GNU Guix and the GNU System distribution." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: guix-devel@gnu.org Errors-To: guix-devel-bounces+larch=yhetil.org@gnu.org Sender: "Guix-devel" X-Migadu-Flow: FLOW_IN ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=yhetil.org; s=key1; t=1618476452; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:in-reply-to:in-reply-to: references:references:list-id:list-help:list-unsubscribe: list-subscribe:list-post; bh=zcgzwRXlfsLeN91OAFsXbaWTU0ldDN6y8IMQ8JPyA7U=; b=cx/5XVFQ3p032q7yFQb09H2KZxt7PpeHdH1/mEvw+yRu726OXbuRqqp6m4EDsfxCxYgycQ u4eQAh2krdMD2AGd2yN06BT0OCEepnMxX+VyH1I8/dtOIgagbhKVRNkl0OXqborLJx8mL+ AQIZTQmG4aMiQv0UVX7EvY2CXFjfSoL942yDg4zsLzUsmADw5HTvu98OFXSKnw4ykhnnfP G1NtL0/oy5AbdMTv9NKmVKrS9NQk4HL5zJ4KGdaW0qtPFa6bDo+pziXcE4Iv1XbvohtkfG 73UNxbmLfrkc6X032g9pijl7IOgvaly6Ri9BtMd4SlVvCHgGsemvPk9hmJKjtQ== ARC-Seal: i=1; s=key1; d=yhetil.org; t=1618476452; a=rsa-sha256; cv=none; b=RGBLsKj0llWKgk6RnXrIkcAonSJxDAY6f7sf/X1+XIcYB3r2OUdKOsZm53neYrpzqskzJg eYJIYaYDFZ/EcwnvswqlSd9K1fbB3BoujgtGHnRmUiA+A7MB4fvQsdsumBi7O1QXw4gmjx VILvOoQCgYErigSSd152ospe0pLFVafgTjN0NrQQ0SAlFd0eVgbcOJhX537qzJ6csO2Bdh ePE3tOybWS51myYxcyiw8u/66HVPx1zAOfOxPv8I9XPj76uXsH6MB++FpcvVaZpMkk2fOj a0dWlDynaxRwn/ob6Rk7groe14go23mYgAl4FqGh7EDTbnS6utexZuOX37BGRQ== ARC-Authentication-Results: i=1; aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Spam-Score: -2.04 Authentication-Results: aspmx1.migadu.com; dkim=none; dmarc=none; spf=pass (aspmx1.migadu.com: domain of guix-devel-bounces@gnu.org designates 209.51.188.17 as permitted sender) smtp.mailfrom=guix-devel-bounces@gnu.org X-Migadu-Queue-Id: 4CFB1245CF X-Spam-Score: -2.04 X-Migadu-Scanner: scn0.migadu.com X-TUID: 704wK/NFsXri --=-=-= Content-Type: text/plain Content-Transfer-Encoding: quoted-printable Luciana Lima Brito writes: > I implemented a basic json output for the derivation comparison page, > for my first contribution as an Outreachy applicant. > > The patch for the code I've changed is attached. > I'm waiting your reviews :) Hi Luciana, I'm not quite sure how to apply this, I'd suggest using git format-patch to generate the file next time as I think there would normally be some metadata along with the diff. Looking at the diff though: > diff --git a/guix-data-service/web/compare/controller.scm b/guix-data-ser= vice/web/compare/controller.scm > index a6aa198..b7788cb 100644 > --- a/guix-data-service/web/compare/controller.scm > +++ b/guix-data-service/web/compare/controller.scm > @@ -584,19 +584,115 @@ > (derivation-differences-data conn > base-derivation > target-derivation))))) > - (case (most-appropriate-mime-type > - '(application/json text/html) > - mime-types) > - ((application/json) > - (render-json > - '((error . "unimplemented")) ; TODO > - #:extra-headers http-headers-for-unchanging-content)) > - (else > - (render-html > - #:sxml (compare/derivation > - query-parameters > - data) > - #:extra-headers http-headers-for-unchanging-content))))))) > + (let ((outputs (assq-ref data 'outputs)) > + (inputs (assq-ref data 'inputs)) > + (sources (assq-ref data 'sources)) > + (system (assq-ref data 'system)) > + (builder (assq-ref data 'builder)) > + (args (assq-ref data 'arguments)) > + (environment-variables (assq-ref data 'environment-varia= bles)) > + (get-derivation-data > + (lambda (items) > + (map > + (match-lambda > + ((name path hash-alg hash recursive) > + `(,@(if (null? name) > + '() > + `((name . ,name))) > + ,@(if (null? path) > + '() > + `((path . ,path)) > + ) > + ,@(if (or (null? hash-alg) (not (string? hash-a= lg))) > + '() > + `((hash-algorithm . ,hash-alg)) > + ) > + ,@(if (or (null? hash) (not (string? hash))) > + '() > + `((hash . ,hash)) > + ) > + ,@(if (null? recursive) > + '() > + `((recursive . ,(string=3D? recursive "t"= )))))) > + ((derivation output) > + `(,@(if (null? derivation) > + '() > + `((derivation . ,derivation))) > + ,@(if (null? output) > + '() > + `((output . ,output))))) > + ((derivation) > + `(,@(if (null? derivation) > + '() > + `((derivation . ,derivation)))))) > + (or items '()))))) > +=20=20=20=20=20=20=20=20=20=20=20=20 > + (let ((base-system (assq-ref system 'base)) > + (target-system (assq-ref system 'target)) > + (common-system (assq-ref system 'common)) > + > + (base-builder (assq-ref builder 'base)) > + (target-builder (assq-ref builder 'target)) > + (common-builder (assq-ref builder 'common)) > + > + (base-args (assq-ref args 'base)) > + (target-args (assq-ref args 'target)) > + (common-args (assq-ref args 'common))) > + > + (let ((matched-outputs (append-map get-derivation-data > + (list (assq-ref outputs= 'base) > + (assq-ref outputs= 'target) > + (assq-ref outputs= 'common)))) > + (matched-inputs (append-map get-derivation-data > + (list (assq-ref inputs '= base) > + (assq-ref inputs '= target)))) > + (matched-sources (append-map get-derivation-data > + (list (assq-ref sources= 'base) > + (assq-ref sources= 'target) > + (assq-ref sources= 'common))))) I would consider whether it's useful to have all these let blocks, and whether here is the right place for them. Taking a binding like outputs, it's only used in a later let. You can do something like this (with let*) to remove the need to have multiple let blocks. (let* ((outputs (assq-ref data 'outputs)) (matched-outputs (append-map get-derivation-data (list (assq-ref outputs 'base) (assq-ref outputs 'target) (assq-ref outputs 'common)))) Also, since matched-outputs is only used when rendering the JSON, I'd move all the bindings that are only used for the JSON output within that part of the case statement, so that it's clearer that they only apply to that bit of the code. Does that make sense? > + (case (most-appropriate-mime-type > + '(application/json text/html) > + mime-types) > + ((application/json) > + (render-json > + `((revision I'm not sure what revision here referrs to. > + . ((base > + . ((derivation . ,base-derivation))) > + (target > + . ((derivation . ,target-derivation))))) > + (outputs > + . ,((lambda (l) (cond > + ((=3D (length l) 3) `((base . ,(= first l)) > + (target . ,(= second l)) > + (common . ,(= third l)))) > + ((=3D (length l) 2) `((base . ,(= first l)) > + (target . ,(= second l)))) > + (else `((common . ,(first l)))))) > + matched-outputs)) > + (inputs > + . ((base . ,(first matched-inputs)) > + (target . ,(second matched-inputs)))) > + (source > + . ((base . ,(first matched-sources)) > + (target . ,(second matched-sources)) > + (common . ,(third matched-sources)))) > + (system > + . ((common . ,common-system))) > + (builder-and-arguments > + . ((builder . ,common-builder) > + (arguments > + . ((base . ,(list->vector > + base-args)) > + (target . ,(list->vector > + target-args)))))) > + (environment-variables . ,environment-variables)) > + #:extra-headers http-headers-for-unchanging-content)) > + (else > + (render-html > + #:sxml (compare/derivation > + query-parameters > + data) > + #:extra-headers http-headers-for-unchanging-content)= ))))))))) > > (define (render-compare/package-derivations mime-types > query-parameters) I hope that helps, just let me know if you have any questions, Chris --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQKlBAEBCgCPFiEEPonu50WOcg2XVOCyXiijOwuE9XcFAmB3/VRfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldDNF ODlFRUU3NDU4RTcyMEQ5NzU0RTBCMjVFMjhBMzNCMEI4NEY1NzcRHG1haWxAY2Jh aW5lcy5uZXQACgkQXiijOwuE9XepoQ/+JUc0GPCu3/CFIU946vb95f/ZZ989lHXq pdWWy8hRzXf/AbiO9MKNP+WmY5/ympf0bRWq+X5B5qZF4ZstX7knNI/8QAtW3Evz abJH5MPdHi/1ME1zylXqFx1iUvIu1Dp/U3DC+3Mr1Yr493dXN5ZDNFzfsyUHF9oD /s9W+cbdi3ILhCrcutxqRpkIC+vt+WvA8CBtCL6wHb8LpHWM6YM8M95a+AxTSRBm u8iMiuHne2IPqpDRTzz0kSzXt6+GbXqUxIRxW1HK9ysZKgtUYSQvQ9NxY3+AB+VQ QdahRpOtBSN2WUvmsIPrIrQL5qMF3Hsleq1IqmduRvO0vkXPfYl7TRhIPSB9Yfu8 l9kCNA/BaptxFg63rwK1iRJRzJKSMiCdI2CrOyNoyr2F5kI9RBiEVsVDW8IHWQnG 9dj+H6LAnO/H0ixPzHsNeVaExPg781B4q2WjyYqnxE+HllnxrfpwvJUxVkc9sQuz FBfPHwu/+6Ik7m//eu/ro/Oz2NHox6+/QuLJQNqlxE2Alw3o1cWCTgKeI4t//h8z K3jwl67AOvJTHD9fWg2a3ramdTFQOfnKEhL3b1qddj6vPnk8slgzWX9ZlRSPoHIQ cJvRx2QDxpLCeNb+py7d/bAhvQDXRo42KQqv9lvubVrTWj724EggSGowG5QQTn+9 cDixPwbc6+U= =Bny/ -----END PGP SIGNATURE----- --=-=-=--