Source path assumptions should be documented

[michaelklishin]

Joplin's "create migration" task uses fairly naïve assumptions about source directory layout for code-driven migrators (e.g. Cassandra). It strips off the first path part and turns the rest into a namespace name. This means if you try to use src/joplin instead of joplin, you'll get compilation errors that are not very pleasant to debug.

I think this at least should be documented, and maybe even made optional configurable with a function defined with a migrator attribute.

[martintrojer]

It's supposed to create the migration file(s) in the path given for that migrator target. So if that configuration is correct, everything should work fine.

https://github.com/juxt/joplin/blob/master/joplin.core/src/joplin/core.clj#L171

You should not create new migrations with paths part of their names, like 'lein joplin create foo/bar/baz/miy-migration" -- joplin expects a flat file structure inside the configured folder.

[michaelklishin]

That wasn't my point. Since namespace names should correspond to paths on the classpath, Joplin-generated migrations have to make some assumptions about ns names w/o knowing anything about source paths configuration. And the assumption made at the moment is

1. Not very prominent in the docs
2. Pretty simplistic

e.g. I couldn't get it to work with src/joplin being one of my source directories, which I'd expect to be a pretty common case.

[tomconnors]

I just got bit by this and thought I'd leave my solution here for anyone else who runs into it. My source layout is a bit more nested than usual clj projects. My migrations live in namespaces starting with kc.admin.server.migrations, which are located in the directory src/admin/server/kc/admin/server/migrations. This means that joplin wants my namespaces to be called admin.server.kc.admin.server.migrations. The fix is to redefine the function that gets the namespace from the :migrator, joplin.core/drop-first-part. For me, this looks like:

(defn drop-first-part* [path delimiter]
  (->> (string/split path #"/")
       rest
       (drop 2) ;; This is the change from the original implementation of this function.
       (interpose delimiter)
       (apply str)))
;; later...
(with-redefs [joplin.core/drop-first-part drop-first-part*]
    (joplin/migrate-db migration-conf))

Perhaps a fix the library could make is to allow :migrator to be a map of {:ns _ :path _}, and when :ns is provided, use that rather than calling drop-first-part. @martintrojer would you take a PR for that?

[martintrojer]

Sounds like a good idea, I'd be happy to merge

[metametadata]

I've just stumbled upon the same issue trying to put all joplin stuff inside backend/src/app/joplin/ instead of joplin/.

I could get it to create migration files by providing :migrators {:jdbc-mig "backend/src/app/joplin/migrators/jdbc"} in joplin.edn, but then migrate command is not able to locate classfiles for the created migrations:

Exception in thread "main" java.io.FileNotFoundException: Could not locate src/app/joplin/migrators/jdbc/20161225185836_foobar__init.class or src/app/joplin/migrators/jdbc/20161225185836_foobar.clj on classpath. Please check that namespaces with dashes use underscores in the Clojure file name., compiling: ...

[nha]

Same issue, I used tomconnors fix and have to require every migration as well for it to work.