Skip to content

Commit

Permalink
Temp targets #88 address all of Aaron's code review items
Browse files Browse the repository at this point in the history
#106
Standardize on name "temp targets"
Use -> for cleaner code
Add comments for ramifications of error when deleting temp target
Also: made sure temp target testing is in the regtest suite
  • Loading branch information
Myron Ahn committed Nov 14, 2013
1 parent 5359d4e commit 9f3e15a
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 17 deletions.
3 changes: 2 additions & 1 deletion resources/regtest/run-all.sh
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@ if ($(dirname $0)/regtest_fs.sh &&
$(dirname $0)/regtest_inputs_outputs.sh &&
$(dirname $0)/regtest_methods.sh &&
$(dirname $0)/regtest_protocol_eval.sh &&
$(dirname $0)/regtest_s3.sh); then
$(dirname $0)/regtest_s3.sh &&
$(dirname $0)/regtest_temp.sh); then
echo "run-all: ALL TESTS PASSED"
else
echo "run-all: SOME TESTS FAILED"
Expand Down
36 changes: 20 additions & 16 deletions src/drake/core.clj
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,13 @@
temp-outputs (step :temp-outputs)
temp-input-map-lookup (parse-tree :temp-input-map-lookup)
[existing-temp-outputs empty-temp-outputs] (split-with data-in? temp-outputs)
empty-temp-output-step-deps (map (comp temp-input-map-lookup normalized-path) empty-temp-outputs)
empty-temp-output-step-deps (flatten empty-temp-output-step-deps)
empty-temp-output-step-deps (filter step-set empty-temp-output-step-deps)
expanded-temp-outputs (map (comp (partial expand-outputs parse-tree step-list) steps)
empty-temp-output-step-deps)
expanded-temp-outputs (flatten expanded-temp-outputs)
]
empty-temp-output-step-deps (->> empty-temp-outputs
(map (comp temp-input-map-lookup normalized-path))
flatten
(filter step-set))
expanded-temp-outputs (->> empty-temp-output-step-deps
(map (comp (partial expand-outputs parse-tree step-list) steps))
flatten)]
(trace "expand-outputs step:" step)
(trace "expand-outputs step-list:" step-list)
(trace "expand-outputs existing-temp-outputs:" existing-temp-outputs)
Expand Down Expand Up @@ -617,22 +617,26 @@
(when (not (empty? trimmed-deps))
(future
(try
(trace "Running future for file" file "deps" trimmed-deps)
(trace "Running future to delete target:" file "dependencies:" trimmed-deps)
(let [successful-deps-count (reduce +
(map (fn [i]
@((steps-map i) :promise))
trimmed-deps))]
(trace "Finished waiting for dependents of file"
(trace "Finished waiting for dependents of target:"
file
"deps"
"dependencies:"
trimmed-deps
"successful-count"
"successful-count:"
successful-deps-count)
(when (= successful-deps-count (count trimmed-deps))
(info "Deleting temp file:" file)
(info "Deleting temp target:" file)
(fs di/rm file)))
(catch Exception e
(error e "Future for file" file "Caught exception")))))))))
; Likely to happen if there is some problem with deleting the file.
; Catch the exception and inform the user of the problem,
; but do not hald execution as deletion of the file is probably not
; critical to the workflow.
(error e "Exception deleting temp target:" file)))))))))

(defn- run-steps-async
"Runs steps asynchronously.
Expand Down Expand Up @@ -662,7 +666,7 @@
assoc-promise
(assoc-function parse-tree))]

(when (not (:keep-temp-files *options*))
(when (not (:keep-temp-targets *options*))
(setup-temp-deleting-futures parse-tree steps-future))

(post event-bus (EventWorkflowBegin steps-data))
Expand Down Expand Up @@ -995,8 +999,8 @@
"Turn on even more verbose debugging output.")
(no-arg version
"Show version information.")
(no-arg keep-temp-files
"Do not auto-delete temp files")
(no-arg keep-temp-targets
"Do not auto-delete temp targets")
(with-arg tmpdir
"Specifies the temporary directory for Drake files (by default, .drake/ in the same directory the main workflow file is located)."
:type :str
Expand Down

1 comment on commit 9f3e15a

@dirtyvagabond
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Please sign in to comment.