Skip to content
New issue

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

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

Already on GitHub? # to your account

pools.NewResourcePool function signature divergence #56

Closed
clover-bt opened this issue Aug 12, 2019 · 1 comment · Fixed by #58
Closed

pools.NewResourcePool function signature divergence #56

clover-bt opened this issue Aug 12, 2019 · 1 comment · Fixed by #58

Comments

@clover-bt
Copy link

NewResourcePool from the github.com/vitessio/vitess/go/pools module is called from https://github.com/ThalesIgnite/crypto11/blob/8367a3ec46bf7e0250f82f1e1c3a45bc43e9112f/crypto11.go#L320. However, the function signature does not match https://github.com/vitessio/vitess/blob/b5eb51a6b17f17af8b2002187fdaf653ef47cf96/go/pools/resource_pool.go#L88, which appears to have changed May 2019: vitessio/vitess@9cebb40#diff-0f7dd748d05e2a8a4b9295ff9bfc3c3a.

@dmjones
Copy link
Contributor

dmjones commented Aug 13, 2019

You are not the first to mention this: #51

I believe we are tripping over the way Go treats packages that are tagged v2.0.0 or higher, but don't use modules (reference). Go makes a sweeping assumption that semver is not in use and treats every version as backwards compatible.

So if you have run go get github.com/vitessio/vitess in your project, or if another dependency relies on vitess v3, Go will assume it's all compatible.

The short fix answer is probably to rekindle #51 and update to v3.x of vitess.

The longer fix, which I'll open an issue for, should be to clone the interesting parts of vitess and make it available as a separate Go module in Github. It was a very heavy dependency just to get a resource pool implementation.

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

Successfully merging a pull request may close this issue.

2 participants