dolthub / go-mysql-server

A MySQL-compatible relational database with a storage agnostic query engine. Implemented in pure Go.
Apache License 2.0
2.36k stars 209 forks source link

Remove Consul Dependency #177

Closed tooolbox closed 4 years ago

tooolbox commented 4 years ago

The first primary use case of go-mysql-server is:

Stand-in for MySQL in a golang test environment, using the built-in memory database implementation.

When I imported it into my project, it errored due to an ambiguous import, as go-mysql-server imported a different version of Consul (???) than my project. Here's the essential bits of the log:

go: downloading github.com/liquidata-inc/go-mysql-server v0.6.0
go: found github.com/liquidata-inc/go-mysql-server in github.com/liquidata-inc/go-mysql-server v0.6.0
go: downloading github.com/liquidata-inc/vitess v0.0.0-20200807222445-2db8e9fb6365
go: downloading github.com/hashicorp/golang-lru v0.5.3
go: downloading github.com/oliveagle/jsonpath v0.0.0-20180606110733-2e52cf6e6852
go: downloading github.com/mitchellh/hashstructure v1.0.0
go: downloading github.com/hashicorp/consul v1.4.0
# my/project
package my/project
    imports github.com/abronan/valkeyrie/store/consul
    imports github.com/hashicorp/consul/api: ambiguous import: found package github.com/hashicorp/consul/api in multiple modules:
    github.com/hashicorp/consul v1.4.0 (/Users/tooolbox/go/pkg/mod/github.com/hashicorp/consul@v1.4.0/api)
    github.com/hashicorp/consul/api v1.1.0 (/Users/tooolbox/go/pkg/mod/github.com/hashicorp/consul/api@v1.1.0)

I imagine that go-mysql-server isn't actually using the Consul API anywhere, it's just an accident of the dependency tree, but I think it's a tad overboard. If anything can be done to remove this dependency it would be appreciated.

zachmu commented 4 years ago

Thanks for pointing out that this is causing you problems. That particular dependency is brought in by vitess, which implements the parser / server logic (as opposed to the execution engine that go-mysql-server provides).

We only use a small fraction of vitess, and we will be paring it down to get rid of this and other unused dependencies. In the meantime, you can solve this by introducing a replace directive in your project's go.mod.

tooolbox commented 4 years ago

Okay great, good to know.

Unfortunately I can't use a replace directive to rewrite one of those paths to the other; Go says something about not being able to have two different import paths point to the same package (sorry, I didn't save the error). Of course, I may have done it incorrectly. Either way, I appreciate it.

zachmu commented 4 years ago

This is gone as of the the latest master. Haven't cut a release yet, waiting until I get triggers fully working.

tooolbox commented 4 years ago

Wow, great! Thanks for hitting this ❤️

zachmu commented 4 years ago

Cool, let us know if you find any other problems!