From 3df8feb3a2b30b733b141f6e6e43572998311524 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 5 Dec 2023 11:27:52 -0500 Subject: [PATCH] Add TODO about setting `null` instead of empty string in JSON An empty string is a sneaky way to avoid hard failures --- things that expect strings still get strings, but it does conversely open the door up to soft failures (spooky-action-at-a-distance ones because the string did not have the expected invariants). "Fail fast" with null will ultimately make the system more robust, but force us to fix more things up front, and I don't want to change this without also fixing those things up front, especially as this commit is for now just part of the the preparatory PR for which this is dead code. --- src/hydra-eval-jobs/hydra-eval-jobs.cc | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/hydra-eval-jobs/hydra-eval-jobs.cc b/src/hydra-eval-jobs/hydra-eval-jobs.cc index b44ccbb3..2794cc62 100644 --- a/src/hydra-eval-jobs/hydra-eval-jobs.cc +++ b/src/hydra-eval-jobs/hydra-eval-jobs.cc @@ -250,6 +250,10 @@ static void worker( // See the `queryOutputs` call above; we should // not encounter missing output paths otherwise. assert(experimentalFeatureSettings.isEnabled(Xp::CaDerivations)); + // TODO it would be better to set `null` than an + // empty string here, to force the consumer of + // this JSON to more explicitly handle this + // case. out[outputName] = ""; } }