feat(connection): deprecate sql server 2008

[javiertury]

Deprecate sql server 2008 which will become completely unsupported by Microsoft in July 9, 2019. This will helps us clean sequelize code by removing some mssql specific hacks in the future.

The versionSupport method can be easily extended to other dialects. Additionally, use semver.coherce for robustness. Some DBs that report incomplete non-conforming semver versions (e.g. 9.5 instead of 9.5.0).

Related to issues #10284 #10285

[codecov]

Codecov Report

Merging #10704 into master will increase coverage by 21.82%.
The diff coverage is 93.75%.

@@             Coverage Diff             @@
##           master   #10704       +/-   ##
===========================================
+ Coverage   74.54%   96.36%   +21.82%     
===========================================
  Files          85       94        +9     
  Lines        8186     9028      +842     
===========================================
+ Hits         6102     8700     +2598     
+ Misses       2084      328     -1756

Impacted Files	Coverage Δ
lib/dialects/mssql/index.js	100% <ø> (ø)	⬆️
lib/dialects/abstract/index.js	100% <ø> (ø)	⬆️
lib/utils/deprecations.js	100% <100%> (ø)	⬆️
lib/sequelize.js	96.33% <100%> (+8.27%)	⬆️
lib/dialects/abstract/connection-manager.js	90.98% <91.66%> (+2.48%)	⬆️
lib/associations/belongs-to-many.js	97.93% <0%> (ø)	⬆️
lib/hooks.js	96.96% <0%> (ø)	⬆️
lib/dialects/mariadb/connection-manager.js	100% <0%> (ø)
lib/dialects/mariadb/index.js	100% <0%> (ø)
lib/dialects/sqlite/index.js	100% <0%> (ø)
... and 35 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30c5ca5...9ee6105. Read the comment docs.

[sushantdhiman]

LGTM, but breaking. Rather than throwing error we should show deprecation warning.

[javiertury]

@sushantdhiman, now only a warning is shown

[sushantdhiman]

This needs some cleanup

1. Define a new key minimumSupportedVersion: undefined here
2. For MSSQL set that to 11.0.0 here
3. After version is acquired you can check if it is less than minimumSupportedVersion here, given that minimumSupportedVersion is not empty
4. For now a deprecation warning from here will be used, in next version we can throw an error

Once you update with master you will notice there is some duplication with this certain code here and here. We should move this is utils perhaps and add some more unit tests against this. With that there is no need to parse version here

[javiertury]

Done. Finally I discarded the test about the warning message. The reason is that I can't spy on deprecation warning functions using sinon. If later we use errors for unsupported DB versions, I would be able to make tests again.

[SimonSchick]

We can't use a single deprecation code fo for multiple versions/dialects.

[javiertury]

I'm in a deadlock. On the one hand @SimonSchick, you are pushing for dialect/version specific warnings. On the other hand @sushantdhiman, you are pushing for an abstracted solution using minimumSupportedVersion.

I can do either

// lib/dialects/mssql/connection-manager.js
const { noMssqlBelow2012 } = require('../../utils/deprecations');

versionSupport() {
    const version = _.get(this.sequelize, 'options.databaseVersion', 0);

    if (semver.valid(version) && semver.lt(version, '11.0.0')) {
      noMssqlBelow2012();
    }
}

Or

// lib/dialects/abstract/connection-manager.js
const { noUnsupportedVersion } = require('../../utils/deprecations');

  versionSupport() {
    const version = _.get(this.sequelize, 'options.databaseVersion', 0);
    const minVer = this.dialect.supports.minimumSupportedVersion;

    if (minVer && semver.valid(version) && semver.lt(version, minVer)) {
      noUnsupportedVersion(this.dialectName, version, minVer);
    }
  }

Also, the more I look at util.deprecate, the more I think it's not the appropriate tool for this job. I think the purpose of util.deprecate and therefore lib/utils/deprecations.js is to deprecate API function calls[0]. Using util.deprecate to print warnings conditional on a remote response(DB version) is stretching too much.

[0]: Docs for util.deprecate

[sushantdhiman]

Closing this PR in favor of #12218, Thanks for this work :)