Make util/with-temp more flexible

[alehatsman]

Hello,

I just started my usage of lambdacd and noticed that macro you have in util namespace is limited.

(defmacro with-temp
  "evaluates the body, then deletes the given file or directory.
  returns the result of the evaluation of the body"
  [f body]
  `(try
     (let [result# ~body]
       result#)
     (finally
       (fs/delete-dir ~f LinkOption/NOFOLLOW_LINKS))))

What i want to suggest is to make it work with multiple forms, not just one.

(defmacro with-temp
  "evaluates the body, then deletes the given file or directory.
  returns the result of the evaluation of the body"
  [f & body]
  `(try
     ~@body
     (finally
       (fs/delete-dir ~f LinkOption/NOFOLLOW_LINKS))))

So that i can use it like here:

(defn -main [& args]
  (let [home-dir (util/create-temp-dir)
        config   {:home-dir home-dir
                        :name     "autoscanner cd"}
        pipeline (lambdacd/assemble-pipeline pipeline/pipeline-def config)
        app      (ui-selection/ui-routes pipeline)]
    (util/with-temp home-dir
      (log/info "LambdaCD Home Directory is " home-dir)
      (runners/start-one-run-after-another pipeline)
      (http-kit/run-server app {:port 8080}))))

Do i miss something? Should i prepare pr?

Best regards, Aleh

[alehatsman]

Ohh i was using 0.13.5, i think we should update project template. I have just realized that that namespace is not public anymore. :/

[flosell]

Hi, thanks for the feedback!

As you noticed, I was quite radical in moving utility functions out of the public namespace. I felt that the public API was cluttered with functions that weren't specific to LambdaCDs purpose but rather implementation details. By keeping them out of the public API, I was more flexible to change them when necessary without worrying about compatibility.

For the temp-dir functionality I'd be open for a discussion about moving them back into the public namespace. I still feel like LambdaCD is not the ideal place for such functionality. I would have hoped that other libraries (e.g. Raynes/fs) would provide something like this but didn't find anything. On the other hand, I see that lots of LambdaCD libraries and users use these shorthands quite heavily so having it in here might make sense.

What do you think (others watching this thread, please chime in as well)?

[alehatsman]

Hello, thank you for the response!

Actually, I agree with you on point about the fact that that namespace and functionality should not be a part of lambdacd.

The only thing is that when you create project or dive in the documentation, you see code snippets with the usage of temp namespace. Because you need to set up home dir.

We can think about moving that code to project like lambda-fs. But only if there will be more of related functionality in future.

So basically I am not sure what is the right way here. Need to think later.

[flosell]

Hmm, I think for a new separate project there's not enough functionality...

Regarding the home-dir specifically, I expected most "real" deployments to use a permanant directory so that the history is kept across restarts. The temp was there mostly for convenience to have a placeholder that works out of the box.

[alehatsman]

Yes, I agree that there's not enough functionality. Then I think that it is a good decision to move it to the internal namespace. Because actually, it's just a few functions. Users can just copy it, I think it's not a big problem.

But anyway, let's apply my changes to it. It works absolutely in the same way, but you can put many forms to it.

[flosell]

closed via #183