Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Source path assumptions should be documented #8

Open
michaelklishin opened this issue Sep 14, 2014 · 6 comments
Open

Source path assumptions should be documented #8

michaelklishin opened this issue Sep 14, 2014 · 6 comments

Comments

@michaelklishin
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor

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

@metametadata
Copy link

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
Copy link
Contributor

nha commented Oct 1, 2017

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants