Feature/storage consul

[jrxFive]

Added a storage option for consul. I probably do need some more thorough testing with exceptions, and had a couple issues with godeps dependencies. Please let me know what additional changes need to be done! Thanks

[jonathanrcross]

not sure whats going on here, when I create a new GOPATH and do godep restore and run cd job/storage/consul; go test . it works, but trying cd job/storage/consul; godep go test .

[...m/ajvb/kala/job/storage/consul]$ go test .                                                                                                                 ) ✗[master][system]
ok      github.com/ajvb/kala/job/storage/consul 7.180s

[...m/ajvb/kala/job/storage/consul]$ godep go test .                                                                                                           ) ✗[master][system]
# github.com/hashicorp/consul/consul/structs
../../../Godeps/_workspace/src/github.com/hashicorp/consul/consul/structs/operator.go:12: undefined: raft.ServerID
../../../Godeps/_workspace/src/github.com/hashicorp/consul/consul/structs/operator.go:19: undefined: raft.ServerAddress
FAIL    github.com/ajvb/kala/job/storage/consul [build failed]
godep: go exit status 2

[ajvb]

Yasss, I'm a huge fan of Consul and have been using it heavily at work. Didn't even think of this. Let me give it a read through.

[jrxFive]

hey @ajvb, made the changes requested. I've been running this a bit in development for a project, I noticed the https://github.com/ajvb/kala/blob/master/main.go#L25 resolution is pretty low, PUTs are the most expensive operation for a distributed KV, since the cache is always attempting to persist and for each job it could potentially get expensive, I personally upped it to 60. Was thinking this should also be a flag? I'd be happy to make it another PR ( I want that hacktoberfest shirt :D ). Thanks again! Please let me know what to try to do about godeps for CI testing.

[ajvb]

Hey @jrxFive

I've been running this a bit in development for a project, I noticed the https://github.com/ajvb/kala/blob/master/main.go#L25 resolution is pretty low, PUTs are the most expensive operation for a distributed KV, since the cache is always attempting to persist and for each job it could potentially get expensive, I personally upped it to 60.

Very good point.

Was thinking this should also be a flag? I'd be happy to make it another PR ( I want that hacktoberfest shirt :D ). Thanks again!

Love it, issue created: https://github.com/ajvb/kala/issues/131

Please let me know what to try to do about godeps for CI testing.

I'm still not exactly sure why the diff for this PR is so big. Did you add some missing dependencies to godeps?

Also, the CI error:

# github.com/hashicorp/consul/consul/structs
../../hashicorp/consul/consul/structs/operator.go:12: undefined: raft.ServerID
../../hashicorp/consul/consul/structs/operator.go:19: undefined: raft.ServerAddress

Makes me believe that CircleCI isn't getting github.com/hashicorp/raft

[jrxFive]

I'm still not exactly sure why the diff for this PR is so big. Did you add some missing dependencies to godeps?

Im not sure either, I do see some libraries I would think would be in the Godeps.json from the current master branch thats missing that it picked up and included.

github.com/garyburd/redigo github.com/rafaeljusto/redigomock

I do see the file its complaining about it in _workspace https://github.com/jrxFive/kala/blob/c6be1bdb49f754aaad1161b0ee475db127c76f9a/Godeps/_workspace/src/github.com/hashicorp/consul/consul/structs/operator.go

# github.com/hashicorp/consul/consul/structs
../../hashicorp/consul/consul/structs/operator.go:12: undefined: raft.ServerID
../../hashicorp/consul/consul/structs/operator.go:19: undefined: raft.ServerAddress

Im going try to rectify these weird additions/removals I still have no idea whats going on with the CI error though =(. Should we close this PR and make another? Or is it okay to try to attempt those changes here?

[ajvb]

I do see the file its complaining about it in _workspace

Right, but it's complaining about raft.ServerID being undefined, which comes from the hashicorp raft library.

Im going try to rectify these weird additions/removals I still have no idea whats going on with the CI error though =(.

1. Try removing everything that isn't your direct kala change and see what happens with the build.
2. Are tests passing locally for you?

Should we close this PR and make another?

I'd say lets figure out what's going on then we can decide. Might be a cleaner merge in the end to start afresh.

Or is it okay to try to attempt those changes here?

Absolutely, push as many builds as you'd like.

[ajvb]

Hey @jrxFive, I'd really like to merge this, but we still have some outstanding issues. Any idea when/if you will be able to fix this up?

Thanks again for all of your work so far.

[jrxFive]

hey @ajvb, I've been able to replicate the issue with godeps go test locally but unfortunately I have no idea how to fix it =(. I will try for this weekend again but don't want to have a stale PR open for such a long time, I'll try to follow up on the results of that. Sorry for the delay and thanks again for all the help!

[gwoo]

I added support for Consul as part of #157. I could of merge this first but honestly, overlooked it. Thanks for the PR! I did review to make sure we had similar approaches and indeed they were almost identical.