-
Notifications
You must be signed in to change notification settings - Fork 79
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
Execute alterTable statements in order #227
Conversation
async.each() in alterTable executes the statements in parallel, allowing for things like creating an index before dropping it. Change async.each to async.eachSeries in order to execute the statements in proper order.
Can one of the admins verify this patch? To accept patch and trigger a build add comment ".ok\W+to\W+test." |
@strongloop/sq-lb-apex , there's an existing PR loopbackio/loopback-connector#168. If it's fixed in |
Yes, we need it because the .each logic is in the alterTable function of this connector, which is called by autoupdate in loopback-connector. Also, many of the connectors override automigrate/autoupdate from loopback-connector. In many cases, each vs. eachSeries isn't critical because it's just the order of the models. In this case, it's critical because it's the order of SQL statements. For example, an index has to be dropped before it's created. |
@ataft, thanks for your contribution. Could you please add some tests to verify your changes? There's a similar PR: loopbackio/loopback-connector#170 which can be used as your reference. Also, please fix the commit linter error as well:
Thanks! |
@dhmlau the current test covers this scenario because it does an automigrate (create the table) and then an autoupdate, which will call alterTable. BTW, this test should be failing intermittently, but given that the order is random it'd be hard to catch. Regarding the commit linter error, what exactly is line 3? I only made one simple change to line 343. |
@ataft, the error is about the commit message, not the actual change. It seems like there are 4 lines of commit message?
|
Yeah, not sure how to do that. There is something called word wrap that's pretty great, it wraps text so people can read it. Submitted new PR: #228 |
Thanks. As a future reference, you can also amend the commit message by following the instructions here: https://help.github.com/en/github/committing-changes-to-your-project/changing-a-commit-message |
async.each() in alterTable executes the statements in parallel, allowing for things like creating an index before dropping it. Change async.each to async.eachSeries in order to execute the statements in proper order.
Fixes #180