-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
refactor(core): Add support for implicit schema in postgres migrations #5233
Conversation
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
@@ -239,7 +239,7 @@ export class Start extends Command { | |||
|
|||
try { | |||
// Start directly with the init of the database to improve startup time | |||
const startDbInitPromise = Db.init().catch(async (error: Error) => | |||
await Db.init().catch(async (error: Error) => |
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.
The idea here was to allow db init to happen "async" while other things were happening; not a huge gain I believe, but are there any reasons to remove this behavior?
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.
it wasn't working as expected when i was testing it. it was somehow only waiting until the db connection was ready, but not until all the migrations were done executing.
but, I can revert it, and test it again.
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 don't think it really provides any major gains, but it's just something to watch out for if we see people saying n8n is taking a bit longer to start. I don't believe we'll be seeing those issues frequently since it's a very simple thing.
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. and since nodes are lazy loaded now, there shouldn't be much of a difference in startup times.
* master: fix(editor): Fix save modal appearing after duplicating a workflow (#5247) feat(editor): Adjust Google sign-in button to adhere to the guidelines (#5248) fix(HelpScout Node): Fix tag search not working when getting all conversations (#5239) fix(editor): Do not request workflow data twice when opening a workflow (#5246) fix(editor): Fix the element-ui imports in SettingsLdapView (no-changelog) (#5245) fix(core): Handle missing binary metadata in download urls (#5242) ci: Simplify DB truncate in tests (no-changelog) (#5243) feat(core): Add LDAP support (#3835) fix(core): Upsert credentials and workflows in the import:* commands (#5231) feat(Jira Software Node): Use resource locator component (#5090) ci: Publish n8n docker images to GHCR (#5213) fix(Google Drive Node): Use the correct mimetype on converted downloads (#5240) fix(editor): Prevent workflow execution list infinite no network error (#5230) fix: Extension being too eager and making calls when it shouldn't (#5232) feat(Send Email Node): Overhaul refactor(core): Add support for implicit schema in postgres migrations (#5233) # Conflicts: # packages/editor-ui/src/components/CredentialEdit/CredentialConfig.vue
Got released with |
DB tests