Package and startup managed Metabase process #40
Conversation
| @@ -49,7 +49,7 @@ export const Services: FunctionComponent<ServicesProps> = ({ postgresql, metabas | |||
| </Space> | |||
| </div> | |||
| {/* Render next step */} | |||
| {metabase !== ServiceStatus.Starting && ( | |||
| {postgresql !== ServiceStatus.Starting && ( | |||
There was a problem hiding this comment.
this makes the table list and import GUI not wait for the lengthy Metabase start.
I tried digging for some tips on how to speed up MB, but nothing useful :(.
There was a problem hiding this comment.
Ok, the Admin Panel doesn't need to wait for it, but query tabs will need to wait for the service to finish starting.
| if (aidColumns.length > 0) { | ||
| if (aidColumns.includes(rowIndexColumn)) { | ||
| await client.query(`ALTER TABLE "${tableName}" ADD COLUMN IF NOT EXISTS "${rowIndexColumn}" SERIAL`); | ||
| } | ||
| const aidColumnsSQL = aidColumns.map((aidColumn) => `'"${aidColumn}"'`).join(', '); | ||
| await client.query(`CALL diffix.mark_personal('"${tableName}"', ${aidColumnsSQL});`); | ||
| } else { | ||
| await client.query(`CALL diffix.mark_public('"${tableName}"');`); | ||
| } |
There was a problem hiding this comment.
note that this squashes two bugs:
- the (not used for now, as explained in a previous PR comment) support for multi-AID columns was a series of
mark_personalcalls, which was some brainfart on my part - without calling public tables
mark_public, Metabase wouldn't even see them. I probably didn't everSELECTfrom them when testing and Metabase did that.
| MB_CHECK_FOR_UPDATES: 'false', | ||
| MB_PASSWORD_COMPLEXITY: 'weak', | ||
| MB_PASSWORD_LENGTH: '0', | ||
| MB_PLUGINS_DIR: metabasePluginsDir, | ||
| MB_SEND_EMAIL_ON_FIRST_LOGIN_FROM_NEW_DEVICE: 'false', | ||
| MB_SEND_NEW_SSO_USER_ADMIN_EMAIL: 'false', |
There was a problem hiding this comment.
a bunch of MB settings which I think make sense for us. The password tweaks allow for a quick login, until the time we implement the disactivation of the login page.
There was a problem hiding this comment.
The password simplification is great. Nice find!
There was a problem hiding this comment.
shame that even this combination of settings doesn't allow for an empty password 🤷. It seems a single letter password is as good as it gets
| return; | ||
| } else { | ||
| console.info('Shutting down Metabase...'); | ||
| metabase?.kill(); |
There was a problem hiding this comment.
this actually isn't too graceful on Linux (error code 143 and some stderr output), but for now fixing this isn't a priority
| @@ -49,7 +49,7 @@ export const Services: FunctionComponent<ServicesProps> = ({ postgresql, metabas | |||
| </Space> | |||
| </div> | |||
| {/* Render next step */} | |||
| {metabase !== ServiceStatus.Starting && ( | |||
| {postgresql !== ServiceStatus.Starting && ( | |||
There was a problem hiding this comment.
Ok, the Admin Panel doesn't need to wait for it, but query tabs will need to wait for the service to finish starting.
|
|
||
| export async function shutdownMetabase(metabase?: ChildProcess): Promise<void> { | ||
| if (isWin) { | ||
| // No graceful way to shutdown Metabase on Windows, let the OS handle it. |
There was a problem hiding this comment.
This would be a kill either way, so you can leave the code cross-platform until we find a workaround.
There was a problem hiding this comment.
it turns out, when packaged in an exe the metabase service doesn't shutdown by itself. So I'm forced to fix it in the next PR
| postgresql?.kill(); | ||
| if (isWin) { | ||
| // If we let the OS handle shutdown, it will not be graceful, and next start is in recovery mode. | ||
| asyncExecFile(path.join(postgresBinPath, 'pg_ctl'), ['-w', '-D', dataDirPath, 'stop']); |
There was a problem hiding this comment.
Hm, I'm very confused why and how this works... Signals don't work on Windows, so the process should still be killed abruptly, see the code in pg_ctl here.
I have been playing with this for most of the day and I couldn't find a satisfactory solution from inside node.
There was a problem hiding this comment.
Anyway, this should work on Linux as well, so maybe no need to have separate code paths?
There was a problem hiding this comment.
Oh man, I now got it: kill is defined as a custom call on Windows which does IPC.
So much time I wasted in vain on this...
There was a problem hiding this comment.
I admit, I didn't dig that deep. It worked and seemed like the recommended way (somewhere in pg docs), so I went with it.
Per Linux... I don't remember 😅 . I'm revisiting this somewhat for the next PR, so I'll login back to linux and check there. If we have no issues with using pg_ctl in general, then I have no objections towards a common path for win/linux
pdobacz
force-pushed
the
piotr/start-metabase
branch
from
da57484
to
aab59a6
Compare
Closes #37 and closes #10.