search

josephg / node-foundationdb

Modern Node.js FoundationDB bindings
Other
115 stars 17 forks source link

setAPIVersion(600) throws "API version not supported" #44

Closed ex3ndr closed 4 years ago

ex3ndr commented 4 years ago

After upgrading to 1.0.0 we started to get this error when calling setAPIVersion

josephg commented 4 years ago

What api version are you requesting?

josephg commented 4 years ago

... and what version of foundationdb are you connecting to?

ex3ndr commented 4 years ago

It is in the title =) Requested 600, FDB version was 6.0.15. Updating values to 620 and 6.2.20 works though.

josephg commented 4 years ago

Yep. Frankly the version API kinda sucks here. You have to pick a version >= 620 (the version requested by the headers as of 1.0.0) but <= the actual version of foundationdb you’re using. More notes here. Which makes supporting new foundationdb features a bit of a mixed blessing. And means 1.0.0 of this library requires foundationdb 6.2.0+. (Which in my defence has been out for at least a year or so)

I’m pretty sure there are ways to add support for newer versions of the api without also losing support entirely for older versions of fdb but I’m not sure how, and I think that would require a really big testing environment on my end with different versions of foundationdb itself - which is something I’m not committed enough to set up.

You can sort of override it by calling setAPIVersion(600, 600). But the C API for opening a database changed at some point in the intervening versions, so I’m not sure what will happen if you try that.

ex3ndr commented 4 years ago

Jeez, thank god we upgraded FDB right before migration to new library and not otherwise! May be just include something like this in the docs?

josephg commented 4 years ago

Yeah fair point. Should have been clearer about that, although I assume you'll run tests before deploying straight to production. Anyway, added a note in both the readme and changelog.

The reason I bumped the minimum API version at the same time as hitting 1.0 is because of semver - I won't bump the minimum database version without also bumping the semver major version. And I sat on #23 for over a year.

But yeah, good reminder to make that super clear in the documentation going forward.