Neo4J Support in Go Migrate [WIP] #1
Conversation
|
Some current issues I am dealing with:
|
|
The Driver definition file mentions that you can return |
|
@ACPrice and @jomarquez |
| } | ||
|
|
||
| func isReady(ctx context.Context, c dktest.ContainerInfo) bool { | ||
| ip, port, err := c.Port(7687) |
There was a problem hiding this comment.
This is for a test, the function asks the docker test suite which docker port has been mapped to 7687, which is the bolt exposed port in the neo docker container. In real usage, the port would be provided by the URL the user gives
|
|
||
| func (n *Neo4J) Unlock() error { | ||
| return nil | ||
| } |
There was a problem hiding this comment.
what are these? Just to fulfill interface methods?
doesn't seem like we need a sync.Mutex for Neo4J struct.
Also now that I see it, did you mean to have capital J for Neo4J?
There was a problem hiding this comment.
Yes, there is a generic Driver interface that go migrate has you fill out, and requests you return nil if the database isn't capable of locking (which Neo isn't).
You are correct about the J, I have replaced it with lowercase
database/neo4j/neo4j.go
Outdated
| defer session.Close() | ||
|
|
||
| _, err = session.Run("MERGE (sm:$migration {version: $version, dirty: $dirty})", | ||
| map[string]interface{}{"migration": n.config.MigrationsLabel, "version": version, "dirty": dirty}) |
There was a problem hiding this comment.
Hm with this, wouldn't it be possible to get two nodes with same version but differing dirty bool state?
There was a problem hiding this comment.
It shouldn't be because we run a ensureVersionConstraint function during driver creation that enforces a unique constraint on version number
database/neo4j/neo4j.go
Outdated
| version = -1 | ||
| } | ||
|
|
||
| return |
There was a problem hiding this comment.
Since the variable are named in the function signature, they will all be returned with an empty return statement. I have changed it to return explicitly for consistency
My attempt at providing an implementation of migrate for Neo4J. Based on previous work by @axiomzen here
WithInstance. This function should accept an existing DB instance and a Config{} struct and return a driver instance.