diff --git a/src/cuirass/base.scm b/src/cuirass/base.scm index 82f49a4..af24a28 100644 --- a/src/cuirass/base.scm +++ b/src/cuirass/base.scm @@ -20,6 +20,10 @@ ;;; You should have received a copy of the GNU General Public License ;;; along with Cuirass. If not, see . +;; Comment: + +;; This is called base because it interface with guix daemon. + (define-module (cuirass base) #:use-module (fibers) #:use-module (cuirass logging) diff --git a/src/cuirass/database.scm b/src/cuirass/database.scm index 56f421d..ccb8309 100644 --- a/src/cuirass/database.scm +++ b/src/cuirass/database.scm @@ -547,6 +547,7 @@ Assumes that if group id stays the same the group headers stay the same." ;; before those in 'scheduled' state (-2). "status DESC, timestamp DESC") (_ "id DESC"))) + ;; TODO: create macro to read the following SQL query from something-get-builds.sql (stmt-text (format #f "SELECT * FROM ( SELECT Builds.id, Outputs.name, Outputs.path, Builds.timestamp, Builds.starttime, Builds.stoptime, Builds.log, Builds.status, diff --git a/src/cuirass/http.scm b/src/cuirass/http.scm index 2d66ff9..0899cd1 100644 --- a/src/cuirass/http.scm +++ b/src/cuirass/http.scm @@ -117,6 +117,13 @@ (map build->hydra-build builds))) (define (request-parameters request) + ;; REVIEW: this 'parameters' is ambigious in aiohttp. It's called + ;; request.query because the part after ? in the URL is called the "query + ;; string". Also parameters can be passed via the path part of the URL. + + ;; here is what I use + ;; https://github.com/a-guile-mind/guile-web/blob/master/src/web/decode.scm + "Parse the REQUEST query parameters and return them under the form '((parameter . value) ...)." (let* ((uri (request-uri request)) @@ -125,11 +132,14 @@ (map (lambda (param) (match (string-split param #\=) ((key param) - (let ((key-symbol (string->symbol key))) + (let ((key-symbol (string->symbol (uri-decode key)))) (cons key-symbol (match key-symbol - ('id (string->number param)) - ('nr (string->number param)) + ('id (string->number (uri-decode param))) + ;; I don't think this will scale in terms of + ;; kind query pairs where the key part can be + ;; many different things + ('nr (string->number (uri-decode param))) (_ param))))))) (string-split query #\&)) '()))) @@ -138,9 +148,10 @@ ;;; ;;; Web server. ;;; -;;; The api is derived from the hydra one. It is partially described here : +;;; The exposed api is derived from the hydra one. It is partially described +;;; here : ;;; -;;; https://github.com/NixOS/hydra/blob/master/doc/manual/api.xml +;;; https://github.com/NixOS/hydra/blob/master/doc/manual/api.xml ;;; (define (request-path-components request) @@ -217,6 +228,8 @@ (with-critical-section db-channel (db) (db-get-specifications db))))) (("build" build-id) + ;; REVIEW: don't inline that much code in same procedure aka. procedures + ;; for the win1 (let ((hydra-build (with-critical-section db-channel (db) (handle-build-request db (string->number build-id))))) @@ -234,6 +247,7 @@ (let ((uri (string->uri-reference (string-append "/log/" (basename output))))) + ;; create a procedure for that when you will have forms. (respond (build-response #:code 302 #:headers `((location . ,uri))) #:body ""))) @@ -241,7 +255,10 @@ ;; Not entry for BUILD-ID in the 'Outputs' table. (respond-json-with-error 500 + ;; 500 is very strong error, it's must not be used for expected failure + ;; by the way this is rendering logic, it should be in it's own procedure. (format #f "Outputs of build ~a are unknown." build-id))) + (#f (respond-build-not-found build-id))) (respond-build-not-found build-id)))) @@ -251,13 +268,25 @@ (nr (assq-ref params 'nr))) (if nr (respond-json (object->json-string + ;; don't nest here the code. Because http handlers should follow + ;; the following pattern: + ;; + ;; (define (handler request body) + ;; (let ((input (snarf-input request body))) + ;; (shallow-validate-with-exception input) + ;; (deep-validate-with-exception input (db)) + ;; (let ((out (domain-code-goes-here))) + ;; (sxml->response (handler-template out)))) + ;; + ;; Basically, the current code requires top down and bottom up + ;; reading. Declaring intermediate variables helps readbility. (with-critical-section db-channel (db) (db-get-evaluations db nr)))) - (respond-json-with-error 500 "Parameter not defined!")))) + (respond-json-with-error 400 "Parameter not defined!")))) ;; aka. bad request (("api" "latestbuilds") (let* ((params (request-parameters request)) ;; 'nr parameter is mandatory to limit query size. - (valid-params? (assq-ref params 'nr))) + (valid-params? (assq-ref params 'nr))) ;; or exception? (if valid-params? ;; Limit results to builds that are "done". (respond-json @@ -359,7 +388,10 @@ ;; process each client request and then directly go back waiting for the ;; next client (conversely, Guile's 'run-server' loop processes clients ;; one after another, sequentially.) We can do that because we don't - ;; maintain any state across connections. + ;; maintain any state across connections. I don't understand that + ;; comment. Why no state across connections? isn't sqlite that stores + ;; states? Does it mean there is no global mutable in memory data in + ;; cuirass? ;; ;; XXX: We don't do 'call-with-sigint' like 'run-server' does. (let* ((impl (lookup-server-impl 'fiberized)) diff --git a/src/schema.sql b/src/schema.sql index eb0f7e9..769360f 100644 --- a/src/schema.sql +++ b/src/schema.sql @@ -1,5 +1,8 @@ BEGIN TRANSACTION; +-- REVIEW: The name of the table must be all small caps. +-- COMMENT: Some people argue table name must be singular +-- but I disagree, both might work. CREATE TABLE Specifications ( name TEXT NOT NULL PRIMARY KEY, load_path_inputs TEXT NOT NULL, -- list of input names whose load path will be in Guile's %load-path @@ -64,7 +67,7 @@ CREATE TABLE Builds ( log TEXT NOT NULL, status INTEGER NOT NULL, timestamp INTEGER NOT NULL, - starttime INTEGER NOT NULL, + starttime INTEGER NOT NULL, -- REVIEW: why not start_time? stoptime INTEGER NOT NULL, FOREIGN KEY (derivation) REFERENCES Derivations (derivation), FOREIGN KEY (evaluation) REFERENCES Evaluations (id) @@ -75,5 +78,12 @@ CREATE TABLE Builds ( CREATE INDEX Builds_Derivations_index ON Builds(status ASC, timestamp ASC, id, derivation, evaluation, stoptime DESC); CREATE INDEX Inputs_index ON Inputs(specification, name, branch); CREATE INDEX Derivations_index ON Derivations(derivation, evaluation, job_name, system); +-- COMMENT: Indices make the following trade off: slow down writes and take +-- more space (disk and memory) and in prize you get faster reads. +-- COMMENT: you'd better add CREATE INDEX just below the table that they +-- speed up queries for. + COMMIT; + +--