Skip to content
This repository has been archived by the owner on May 14, 2024. It is now read-only.

Initial jar dispatcher #1

Merged
merged 8 commits into from
Oct 12, 2015
Merged

Initial jar dispatcher #1

merged 8 commits into from
Oct 12, 2015

Conversation

fraburnham
Copy link
Contributor

No description provided.

(defn dispatch
[{:keys [nspace func args] :as entry-map} cli-args]
(require nspace)
(apply (ns-resolve nspace (symbol func)) (concat args cli-args)))
Copy link
Contributor

Choose a reason for hiding this comment

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

func should already be a symbol?

Copy link
Contributor

Choose a reason for hiding this comment

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

dev> (type (clojure.edn/read-string "foo"))
clojure.lang.Symbol
dev> (type (clojure.edn/read-string "\"foo\""))
java.lang.String
dev> 

@briprowe
Copy link
Contributor

briprowe commented Oct 8, 2015

I think an example entry-points.edn file in the README would be very helpful.

@briprowe
Copy link
Contributor

briprowe commented Oct 8, 2015

👍

@@ -0,0 +1,20 @@
(ns dispatcher.core
Copy link
Contributor

Choose a reason for hiding this comment

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

Be sure to exclude clojure.core/read-string here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or, don't refer it in the require block.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed refer -> as

In project.clj:
* Set `:main` to `dispatcher.core`

$ java -jar some-service-0.1.0-standalone.jar worker-name [args]
Copy link
Contributor

Choose a reason for hiding this comment

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

Mabye s/worker-name/entry-point/ ?

Copy link
Contributor

Choose a reason for hiding this comment

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

/touch (this got collapsed)

Launches the appropriate worker from an uberjar

## Usage
In the resources directory:
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's be explicit about the resource path, because there's a difference between a "resource path" and a directory in the project.

So, the entry-points.edn file must be at the resource path: /entry-points.edn

This resource path resolves to multiple places in a clojure project's file system layout. Example file paths that would work:

  • /src/entry-points.edn
  • /resources/entry-points.edn
  • /any/other/directory/listed/in/the/project/clj/as/a/resource/path/entry-points.edn

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made resource path explicitly src/resource/

Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't the proper path.

There's a difference between resource path and the path the file exists at in the project. Resource paths are resolved at runtime after everything has been compiled. The are relative to the classloader, and there are multiple trees (each jar, for example). So, the resource path "/foo" will find a file named "foo" in a root node among all these trees (the classloader has some logic to choose which "foo" if there are multiple).

Leiningen automatically adds both the "src" and "resources" directories as trees for every project.

Since "src" is the root of a tree, if we write the "entry-points.edn" file at "src/entry-points.edn", it will get the resource path "/entry-points.edn". Similarly, since "resources" is a root, if we write the file to "resources/entry-points.edn" it will still have the resource path "/entry-points.edn". On the other hand, if the file is saved to the path "src/resource/entry-points.edn" in the project, it will not be found at the resource path "/entry-points.edn". It will be at "/resource/entry-points.edn".

Since the dispatch code is looking up the "entry-points.edn" file using the resource path, there are multiple places where the client project can put the file. And these places aren't known for sure. They depend on how the project.clj is setup.

The only thing that matters, is that the client project has been configured to ensure that the "entry-points.edn" file (wherever it is stored in the project) is resolvable at the proper resource path (which looks like "/entry-points.edn" atm).

So, with all this in mind, I would suggest we simply state that the file will be loaded from the resource path "/entry-points.edn" and let the client project decide where it wants the file to live for itself.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know if I agree with letting the client project decide where the file will live. The services that will be impacted by this have :resource-paths ["src/resource"]. I'd prefer to make adding that line to the project.clj explicit and keeping the path as it is. In my opinion this eliminates needing to know/remember what valid resource paths are, because it is explicity defined in the project.clj. I see that as helping to eliminate confusion. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Can do that, but we should be clear about the resource path that we're expecting the file to be available at, as this is the true interface.

@mayankag
Copy link

mayankag commented Oct 8, 2015

👍 for me. get another 👍 from @briprowe as well.

@fraburnham fraburnham force-pushed the initial branch 2 times, most recently from b9c1e87 to 9b7f4cb Compare October 8, 2015 21:00
@briprowe
Copy link
Contributor

Looks good! 👍

fraburnham added a commit that referenced this pull request Oct 12, 2015
@fraburnham fraburnham merged commit f40c6c7 into master Oct 12, 2015
@fraburnham fraburnham deleted the initial branch October 12, 2015 17:57
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants