New issue

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

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

Already on GitHub? Sign in to your account

Introduced abstraction for getting migrations #2775

Merged
merged 12 commits into from Sep 12, 2018

Conversation

Projects
None yet
4 participants
@JakeGinnivan
Copy link
Contributor

JakeGinnivan commented Aug 20, 2018

This would allow a webpack migration source which is compatible with bundling. As an example:

const migrationConfig = config || {
    directory: path.resolve(__dirname, 'migrations'),
    migrationSource: new WebpackMigrationSource(require.context('./migrations', false)),
}

class WebpackMigrationSource {
    constructor(private migrationContext: __WebpackModuleApi.RequireContext) {}
    /**
   * Gets the migration names
   * @returns Promise<string[]>
   */
    getMigrations() {
        return Promise.resolve(this.migrationContext.keys())
    }

    getMigration(name: string) {
        return this.migrationContext(name)
    }
}

I can now use webpack and bundle my database migration project.

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Aug 20, 2018

FYI @elhigu

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Aug 22, 2018

Just while I think of it, some other reason you may want this:

  1. Custom folder structure, for example #2735 could be implemented as a migration source without a pull request
  2. I have subclassed Migrator to FilterableMigrator which allows me to have _PRODUCT in a migration, then it only gets run for that product (we have an API which is the same codebase and has different seed data per product). This would be a supported way to do what I am doing already.
  3. You could actually support the typescript / other language support with specific migration sources which compile the migration then return the compiled output.

All of the above and whatever other requirements someone may have around where migrations come from, how to filter them out, and the order they run in can be controlled by the consumer.

It would be awesome to get an idea if this will be merged. It also makes it easy to work around issues like #1922

@elhigu
Copy link
Collaborator

elhigu left a comment

Thanks for this. The code and the idea looks nice and clean.

Only thing that is missing is test for actually setting the custom migration source through configuration and documentation for the feature.

Also it seems like a less work to rebase this on top of #2690 so that would need to be done too after it is ready and merged.

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Aug 25, 2018

Will get it done, just wanted to make sure you were happy with the idea in general.

@JakeGinnivan JakeGinnivan force-pushed the JakeGinnivan:configurable-migration-sources branch from 2e6db24 to 84c567b Aug 25, 2018

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Aug 25, 2018

Unfortunately this PR has become a bit more complex after the other PR. When dealing with migrations you now are passing around an object. If you want the migration name you need to call the migrationSource to get the migration name (because different sources will have different structures).

Will get the docs and expand the tests next, initial thoughts welcome. @elhigu

@@ -0,0 +1,65 @@
import fs from 'fs';
import path from 'path';
import Promise from 'bluebird';

This comment has been minimized.

@kibertoad

kibertoad Aug 25, 2018

Collaborator

Considering that this is only used for Promise.all and for promisification, I would suggest to only import promisify function from bluebird and use native for Promise.all, as there was a general inclination to move away from bluebird usage in Knex.

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 25, 2018

Contributor

When are you considering dropping node 6 support? If we have node 8 only then can just use native promisify too.

Will switch.

This comment has been minimized.

@kibertoad

kibertoad Aug 25, 2018

Collaborator

Unfortunately, Node 6 is still about 25% of all production installations, so chances for dropping it in the nearby future are not that great. It is also officially supported till 2019 April.
(I wish we could drop Node 6 and use async/await freely in tests, but alas)

This comment has been minimized.

@elhigu

elhigu Aug 29, 2018

Collaborator

we can drop it in next year April I suppose https://github.com/nodejs/Release


const readDirAsync = Promise.promisify(fs.readdir, { context: fs });

export class FsMigrations {

This comment has been minimized.

@kibertoad

kibertoad Aug 25, 2018

Collaborator

Why not export it as a default?

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 25, 2018

Contributor

Personal preference coming from TypeScript. https://basarat.gitbooks.io/typescript/docs/tips/defaultIsBad.html

Happy to change if that is the preference.

This comment has been minimized.

@kibertoad

kibertoad Aug 25, 2018

Collaborator

It result in slightly less redundant imports,
Quick search on project shows that there are multiple cases of "export default class" and none of "export class", so it does seem to be the project convention.

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 25, 2018

Contributor

Will change

@@ -419,15 +438,22 @@ export default class Migrator {
}

setConfig(config) {
return assign({}, CONFIG_DEFAULT, this.config || {}, config);
const newConfig = assign({}, CONFIG_DEFAULT, this.config || {}, config);

This comment has been minimized.

@kibertoad

kibertoad Aug 25, 2018

Collaborator

Object.assign could be used instead of a Lodash one here.

@@ -419,15 +438,22 @@ export default class Migrator {
}

setConfig(config) {
return assign({}, CONFIG_DEFAULT, this.config || {}, config);
const newConfig = assign({}, CONFIG_DEFAULT, this.config || {}, config);
if ((config && !config.migrationSource) || !newConfig.migrationSource) {

This comment has been minimized.

@kibertoad

kibertoad Aug 25, 2018

Collaborator

Why checking just newConfig is not enough?

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 25, 2018

Contributor

Because of the way tests use this function, I only want to init the migrationSource if it is not on the current config (if there is one) and it's also not specified in the new config.

Just checking new config meant that different tests calling setConfig would not apply.

This comment has been minimized.

@kibertoad

kibertoad Aug 25, 2018

Collaborator

Not sure if I understand. After you call assign({}, CONFIG_DEFAULT, this.config || {}, config);, doesn't newConfig already include everything that config has?

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 25, 2018

Contributor

Let's say the first test does not specify a migrationSource, but sets the directory. The migrationSource will capture the directory, so when setConfig is called with a different directory, but no migrationSource the current migrationSource (with the old directory) is on this.config.

Basically, the tests can't change the migration directory.

This comment has been minimized.

@kibertoad

kibertoad Aug 25, 2018

Collaborator

Looks like we are talking about different things :)
Under which circumstances would "!config.migrationSource" check pass but " !newConfig.migrationSource" would not? (double check seems redundant to me is what I'm saying as one config is derived from the other)

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 26, 2018

Contributor

Hrrmm, looking at this fresh today. I am not sure exactly.

I think I was thinking something along the lines of, if you specify a migration source initially, which is not the default, I don't want it overridden if you call setConfig again.

Either way seems brittle, going to try and figure a better way

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 26, 2018

Contributor

Have re-written logic so it's clearer

@@ -419,15 +438,22 @@ export default class Migrator {
}

setConfig(config) {
return assign({}, CONFIG_DEFAULT, this.config || {}, config);
const newConfig = assign({}, CONFIG_DEFAULT, this.config || {}, config);

This comment has been minimized.

@kibertoad

kibertoad Aug 25, 2018

Collaborator

Are you sure about naming? There is not much new about it, it is mostly a composite or combined one.

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 25, 2018

Contributor

It is going to be the new config was my thinking?

JakeGinnivan added some commits Aug 26, 2018

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Aug 26, 2018

The migration source has this interface (from typescript point of view)

interface MigrationSource<Migration> {
  getMigrations: () => Promise<Migration[]>
  getMigrationName: (migration: Migration) => string
  getMigration: (migration: Migration) => { up: UpFunction, down: DownFunction }
}

Question about promises etc, currently this works fine, getMigrations being async and migrationName and getMigration being sync. Is this too limiting?

I could make them either all returns promises, or wrap the migrationSource to always turn the returned value into a promise so the implementor doesn't have to worry. Thoughts?

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Aug 26, 2018

I think #2756 and #1922 can be solved by doing this (once this PR lands):

return knex.migrate.latest({
  migrationSource: new FsMigrations('./directory', false, ['.js']);
});

Or something similar, and it will be simple enough to create a migration source if the ootb ones are not configurable enough.

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Aug 26, 2018

Ok, @elhigu @kibertoad I think this is all good to go. Let me know if there is anything else to address, or potential issues you can see!

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Aug 26, 2018

Down the track we could also move language compilation into different sources. So you could have a TypeScriptFsMigrationSource which does the compilation.

}

// Ensure a migrationSource is set
if (!newConfig.migrationSource) {

This comment has been minimized.

@kibertoad

kibertoad Aug 26, 2018

Collaborator

This code looks quite redundant. What's the added benefit of Calling F1 with A, B, C if A || B || C set, or calling F1 with A, B if neither are set, if these are full equivalents, as you would simply be passing undefineds that will get resolved to default value in either case anyway?

This comment has been minimized.

@elhigu

elhigu Aug 30, 2018

Collaborator

This method indeed seems unnecessarily complicated... couldn't we just always do

if (!newConfig.migrationSource) {
    newConfig.migrationSource = new FsMigrations(
        newConfig.directory,
        newConfig.sortDirsSeparately,
        newConfig.loadExtensions
    );
}

?

This comment has been minimized.

@elhigu

elhigu Aug 30, 2018

Collaborator

Only difference I can see with those create commands is that for the second one loadExtensions is never passed on... what difference does that make?

This comment has been minimized.

@elhigu

elhigu Aug 30, 2018

Collaborator

Ah yeah, I think now I get it... anyways to me this would have signaled intent of the first if better:

// if custom FsMigrator config was passed new migrationSource must be created
if (config && (config.directory ||
        config.sortDirsSeparately !== undefined ||
        config.loadExtensions)) {
   newConfig.migrationSource = null;
}

if (!newConfig.migrationSource) {
    newConfig.migrationSource = new FsMigrations(
        newConfig.directory,
        newConfig.sortDirsSeparately,
        newConfig.loadExtensions
    );
}

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 31, 2018

Contributor

Ah that is actually quite a nice way of doing it. Invalidate the current if one of the config settings change, then set if not set. Will update!

config.loadExtensions)
) {
newConfig.migrationSource = new FsMigrations(
newConfig.directory,

This comment has been minimized.

@kibertoad

kibertoad Aug 26, 2018

Collaborator

You are checking for values being set on config and then using values from newConfig. Even if this is still going to achieve same purpose, it reads confusingly. Also I still fail to see benefit of the check, considering that the outcome is always the same, we end up creating FsMigrations once with params from config.

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 26, 2018

Contributor

config is the values the user has passed in, newConfig is the merged config from existing, defaults and then user values.

The additional checks are ensuring the intention of the user works properly.

// are used
if (
config &&
!config.migrationSource &&

This comment has been minimized.

@kibertoad

kibertoad Aug 26, 2018

Collaborator

I think this is the only useful check and the logic should be as following: "if newConfig.migrationSource -> noop, else newConfig.migrationSource = new FSMigrationSource". Everything else doesn't seem to result in any meaningful branches.

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 26, 2018

Contributor

But then this wouldn't work

const migrator = new Migrator({ directory: './foo' })
migrator.latest({ directory: './bar' })

Because the ctor would initialise migrationSource, then the call to latest would check to see if newConfig.migrationSource is set. It is, so no-op. Leaving the FsMigrationSource looking at ./foo rather than ./bar.

This comment has been minimized.

@kibertoad

kibertoad Aug 26, 2018

Collaborator

Ah, OK, now your logic makes sense. Could you clarify comment a little bit to make it more explicit that you are accounting for the case where migrationSource configuration has changed between previous config and newly passed one?

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 26, 2018

Contributor

I could add some specific tests around it. Was confusing to write too :P

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 26, 2018

Contributor

Added some comments to clarify

This comment has been minimized.

@JakeGinnivan

JakeGinnivan Aug 26, 2018

Contributor

Hrrm, added in the other PR. Can move it if not planning on merging that one

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Aug 29, 2018

Hey @kibertoad anything else?

@elhigu

This comment has been minimized.

Copy link
Collaborator

elhigu commented Aug 29, 2018

I will also review this in couple of days... or maybe later on today.

@elhigu

elhigu approved these changes Aug 30, 2018

Copy link
Collaborator

elhigu left a comment

Please consider if that setConfig could be written more clearly.

@you-fail-me

This comment has been minimized.

Copy link

you-fail-me commented Sep 4, 2018

This would be really amazing, I'm struggling to package knex migrations service with webpack to run in aws lambda and the current tight bond with file system is very inconvenient for my case

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Sep 5, 2018

@elhigu I have made your changes, sorry about the timing, had a busy week

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Sep 5, 2018

Actually, made them in #2786

I can backport them if you want, but if you like the initial look of that PR I am happy to fix it up after this merges and land it separately?

@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Sep 6, 2018

@JakeGinnivan Could you give me write permissions on your fork so that I could backport it myself? Would be easier to have it in one place.

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Sep 7, 2018

@kibertoad I have backported and also added you in case there is anything else. I am in Australia so I am unsure how much overlap time wise. But will keep an eye on comments today and push anything up quickly

@JakeGinnivan JakeGinnivan force-pushed the JakeGinnivan:configurable-migration-sources branch from 446799e to e92ada0 Sep 7, 2018

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Sep 7, 2018

Struggling to find why some tests are timing out. Will try to get it all sorted, but any help is welcome :)

@elhigu

This comment has been minimized.

Copy link
Collaborator

elhigu commented Sep 7, 2018

Looks like the test fails are coming right now from missing method mergeConfig

@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Sep 7, 2018

@elhigu That solved it for all Nodes but the 10th one. Huh?..

@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Sep 7, 2018

Fun fact - it is Node 10.10.0-specific, passes fine with earlier ones.

@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Sep 7, 2018

@elhigu This block times out on Node 10.10.0:

      return readDirAsync(absoluteDir).then(
(files) => ({ //This then block is never entered
        files,
        configDir,
        absoluteDir,
      }));
    });

If replaced with fs.readdirSync - works perfectly
Since our usage of readDirAsync looks correct, maybe we should lock CI Node version at 10.9.0 for the time being? This looks like an issue in Node and I suspect that our current master is failing as well.

@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Sep 8, 2018

Update: seems to be a compatibility issue between mock-fs and Node.js 10.10.0 rather than just a bug in Node 10.10.0.

@kibertoad

This comment has been minimized.

Copy link
Collaborator

kibertoad commented Sep 11, 2018

@elhigu Aaaaaand after mock-fs have fixed its compatibility problem we are back online. Everything green, everything passes, everything ready to merge!

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Sep 12, 2018

Awesome! Let me know if you need anything from me

@elhigu elhigu merged commit bf1e384 into tgriesser:master Sep 12, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.06%) to 85.663%
Details
@elhigu

This comment has been minimized.

Copy link
Collaborator

elhigu commented Sep 12, 2018

Thank you very much for your co-operation :D

@JakeGinnivan JakeGinnivan deleted the JakeGinnivan:configurable-migration-sources branch Sep 13, 2018

@JakeGinnivan

This comment has been minimized.

Copy link
Contributor

JakeGinnivan commented Sep 13, 2018

No problems, any chance of a @next tagged release or something ? :)

@elhigu

This comment has been minimized.

Copy link
Collaborator

elhigu commented Sep 13, 2018

yes, lots of chances 👍

veikovx added a commit to veikovx/knex that referenced this pull request Sep 17, 2018

Introduced abstraction for getting migrations (tgriesser#2775)
* Introduced abstraction for getting migrations

This would allow a webpack migration source which is compatible with bundling.

* Fixed migration validation with custom migration source

* Fixed issues after rebasing on muti directory PR

* Renamed parameter and fixed error message

* Addressed some PR comments

* Finished comment

* Moved filename extension filtering into fs-migrator

* Added test showing in memory custom migration source

* Cleaned up how to get config

* Fixed failing test

* Hopefully fix tests

* Fix Node.js 10 support in tests

elhigu added a commit that referenced this pull request Sep 26, 2018

Kill queries after timeout for PostgreSQL (#2636)
* Kill queries after timeout for PostgreSQL

* Fix cancellation connection acquiring and test.

* Fix releasing connection in case query cancellation fails

* Add support for native enums on Postgres (#2632)

Reference https://www.postgresql.org/docs/current/static/sql-createtype.html

Closes #394

Signed-off-by: Will Soto <will.soto9@gmail.com>

* Removal of 'skim' (#2520)

* Allow overwriting log functions (#2625)

* Example build of custom log functions
* Handle logger object better for transactions
* Adjust test to ignore sqlite warning message

* Fixed onIn with empty values array (#2513)

* Drop support for strong-oracle (#2487)

* Remove babel-plugin-lodash (#2634)

While in theory, this may reduce the bundle size,
in practice it adds a ton of overhead during startup
due to the number of additional requires. Bundle
size also shouldn't matter for server side modules.

* Add json support to the query builder for mysql (#2635)

* Add json support to the query builder for mysql

refs #1036

Based on #1902

* Clarify supported version

* fix wrapIdentifier not being called in postgres alter column (#2612)

* fix wrapIdentifier not being called in postgres alter column

* add test for wrapIdentifier call in postgres alter column

* add comment regarding issue

* add issue & PR #'s in comment

* Remove readable-stream and safe-buffer (#2640)

* Add json support to the query builder for mysql

refs #1036

Based on #1902

* Clarify supported version

* Use native Buffer and Stream implementations

* fixes 2630 (#2642)

* Timeout errors shouldn't silently ignore the passed errors, but rather reject with original error. Fixes #2582 (#2626)

* chore: add Node.js 10 (#2594)

* chore: add Node.js 10

* chore: trigger new build

* chore: update lockfile

* chore: trigger new build

* fix: use npm i instead of npm ci

* Fix mssql driver crashing (#2637)

* Run SQL Server tests locally running SQL server in docker

* WIP mssql test stuff

* Patched MSSQL driver to not crash knex anymore

* Removed semicolon from rollback stmt for oracle (#2564)

* Remove WebSQL dialect (#2647)

* Add json support to the query builder for mysql

refs #1036

Based on #1902

* Clarify supported version

* Use native Buffer and Stream implementations

* Remove WebSQL dialect

* add homepage field to package.json (#2650)

* Make the stream catch errors in the query (#2638)

* Make the stream catch errors in the query

* Fix another case in which stream doesnt emits error

* Linter stuff

* Remove setTimeout in tests

* Make a test not to check the MySQL error code

* Fix stream error catching for MariaDB and PostgreSQL

* Fix stream error catching in Oracle

* Throw the error after emitting it to the stream

* Throw the error without instantiating a new Error

* Various fixes to mssql dialect (#2653)

* Fixed float type of mssql to be float

* Many tests where postgres test was not actually ran at all

* Migrations to be mssql compatible

Mssql driver doesn't handle if multiple queries are sent to same transaction concurrently.

* Prevented mssql failing when invalid schema builder was executed by accident

Instead of trying to generate sql from broken schema calls, just make exception to leak before query compiling is started

* Fixed mssql trx rollback to always throw an error

Also modified some connection test query to be mssql compatible

* Fixed various bugs from MSSQL driver to make tests run

* Fixed mssql unique index to be compatible with other dialect implementations

* Enable running mssql tests on CI

* Test for #2588

* Updated tests to not be dependend on tables left from previous test rans

* Trying to make mssql server work on travis

* Updated changelog and version

* Drop mariadb support  (#2681)

* Dropped support for mariasql

* ESLint

* Fixed docs to build again

* Fix knex.initialize() and adds test (#2477)

* Fix knex.initialize() and adds test

* Fix knex.initialize() and adds test

* Added test for reinitializing pool after destroy

* Fixed destroy / reinitialization test

* Fixed the fix

* Implement missing schema support for mssql dialect

* chore: Update dependencies. Remove estraverse (#2691)

* Update dependencies. Remove estraverse

* Fix compatibility with new Sinon

* Increase mssql timeout

* Normalized and validated driverNames of test db clients and fixed oracle test setup (#2692)

* Normalized and validated driverNames of test db clients and fixed oracle test setup

* Fixed failed queries from old query building tests which hadn't been ran in ages

* Allow selecting node version which is used to run oracledb docker tests

* Improved sql tester error messages

* Fixed rest of the oracledb tests

* Removed invalid flag from docker-compose

* Print mssql logs if initialization fails

* Fixed syntax error + final tests

* Added restart of failure for mssql DB initialization to try again if server was not ready

* Printout always mssql logs after container is started

* Fixed wait time printing after trying to connect

* Use npm run oracledb:test for testing oracle in travis

* Add Prettier (#2697)

* Add prettier
* Run files through prettier

* Istanbul -> NYC (#2700)

* istanbul -> nyc

* Update mocha

* Enforce code coverage (#2702)

* Enforce code coverage
* Update coverage numbers with current values

* version assignment on base class, copy on tx client, fix #2705

* Update changelog

* v0.15.1

* Added build step to test script. Fixes #2541 (#2712)

* Revert "Added build step to test script. Fixes #2541 (#2712)" (#2714)

This reverts commit 90ed8db.

* Allow oracle failures for now

* Fix issue with select(0) (#2711)

* Fix issue with select(0). Fixes #2658

* Refactor migrator (#2695)

* Refactor migrator

* Fix exports

* Fix ESLint

* Fix migrator

* Fix reference to config

* Split some more

* Fix table builder

* Fix argument order

* Merge branch 'master' of https://github.com/tgriesser/knex into feature/2690-support-multiple-migration-dirs

# Conflicts:
#	src/migrate/index.js
#	test/index.js
#	test/unit/migrate/migrator.js

* Fix #2715 (#2721)

* Fix #2715, explicitly set precision in datetime & timestamp
* Allow for precision in knex.fn.now, mysql time
* Add test for datetime with precision

* Bump changelog

* 0.15.2

* Fix issues with warnPromise when migration does not return a promise. Fixes #2725 (#2730)

* Add tests for multiple union arguments with callbacks and builders (#2749)

* Add tests for multiple union arguments

* Add some callback and raw tests to union unit tests

* Compile with before update so that bindings are put in correct order (#2733)

* Fix join using builder withSchema. (#2744)

* Use Datetime2 for MSSQL datetime + timestamp types (#2757)

* Use Datetime2 for MSSQL datetime + timestamp types

Datetime2 is now the recommended column type for new date work: https://docs.microsoft.com/en-us/sql/t-sql/data-types/datetime-transact-sql?view=sql-server-2017

* Add tests for MSSQL datetime2 changes

* General/document breaking change (#2774)

* Add changelog entry for a breaking change

* Improve entry

* Allow timestamp with timezone on mssql databases (#2724)

* Allow timestamp with timezone on mssql databases

* Change timestamp parameter to use object instead of boolean

* Update dependencies (#2772)

* Feature/2690: Multiple migration directories (#2735)

* Implement support for multiple migration directories

* Add tests for new functionality

* Fix tape tests

* Pass migration directory upwards

* Fix multiple directory support

* Fix bugs

* Rollback correctly

* Fix remaining tests

* Address comments

* #2758: Implement fail-fast logic for dialect resolution (#2776)

* Implement fail-fast logic for dialect resolution, clean-up code around.

* Remove method that was deprecated long time ago

* Address additional comments

* Try addressing comments

* Set client explicitly

* Fix compatibility with older Node versions

* Use columnize instead of wrap in using(). (#2713)

* Use columnize instead of wrap in using().

This is an attempt to fix #2136. Also added an integration test, couldn't find any existing.

* Exclude MSSQL from test as it does not support .using

* Change test to not use subquery

* Change test

* Introduced abstraction for getting migrations (#2775)

* Introduced abstraction for getting migrations

This would allow a webpack migration source which is compatible with bundling.

* Fixed migration validation with custom migration source

* Fixed issues after rebasing on muti directory PR

* Renamed parameter and fixed error message

* Addressed some PR comments

* Finished comment

* Moved filename extension filtering into fs-migrator

* Added test showing in memory custom migration source

* Cleaned up how to get config

* Fixed failing test

* Hopefully fix tests

* Fix Node.js 10 support in tests

* Test for correctly releasing cancel query connection

@JakeGinnivan JakeGinnivan referenced this pull request Sep 28, 2018

Closed

Added initial knex@next types #29256

8 of 9 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment