-
Notifications
You must be signed in to change notification settings - Fork 350
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
Basic Migrations command #494
Conversation
// @kevinswiber @DrMegavolt What's your idea around the standing points? |
d7a538c
to
3823a93
Compare
what is wrong with just running |
I thought it was a requirement in order to give to the user an uniform interface to use the gateway. |
c36e127
to
a791487
Compare
Codecov Report
@@ Coverage Diff @@
## master #494 +/- ##
==========================================
+ Coverage 92% 92.01% +0.01%
==========================================
Files 219 221 +2
Lines 9280 9333 +53
==========================================
+ Hits 8538 8588 +50
- Misses 742 745 +3
Continue to review full report at Codecov.
|
I removed the CLI integration and we'll stick with the default storing mechanism! 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
d898c13
to
331edc0
Compare
@DrMegavolt Please review this one ore time. I've modified the code to use the Also, I've added a test that should check the whole flow. |
let tmpFile; | ||
let userId; | ||
let oldCredential; | ||
before('I insert some credentials by user ID', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the name should be
I insert some credentials by username
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, let's merge
…rations Basic Migrations command
This pull request will introduce a migration mechanism for the gateway leveraging node-migrate@next module by Tj.
Connects #490
Connects #492 closes #492
Standing points:
This module can track the ran migrations using a
.migrate
file or using a custom storage mechanism. Question is whether we want to use the file or store the things in Redis, for example.If we keep them on Redis, you move the DB, you are ok. If you keep them on the file system you might re-run the migrations if you change the server. ✌️
How do we make sure the users will run the migrations? I've checked Kong and they do not care. They rely on the fact that you read the change log and notice you've got a migration to run; therefore you run them. I'd stick with the same mechanism ✌️
Figure out a way to test the effects by this migration
Integrate the migration command with the(we're not doing that)eg
cmd line