Move redis launch to node #118
Conversation
app/src/js/server/util.ts
Outdated
| throw new Error(`Cannot find ${config.data}`) | ||
| } | ||
|
|
||
| if (!(await fs.pathExists(config.itemDir))) { |
There was a problem hiding this comment.
This check happens after the config is supplemented by the default config, so itemDir will always have a value here. Don't we need to verify that the dir exists? Otherwise there may be an error when saving later.
There was a problem hiding this comment.
It may not exist if the user doesn't specify it. It is not necessary unless the user wants to server local data.
There was a problem hiding this comment.
So we should only check the itemDir if the database is local? In the case where database is local but itemDir is not specified, should we throw an error that the default itemDir is missing?
There was a problem hiding this comment.
We can set the default itemDir to empty string. We only check whether the itemDir exists when local FSU’s used and itemDir is not empty.
app/src/js/server/main.ts
Outdated
| }) | ||
|
|
||
| // wait 1 s before trying to connect to the server | ||
| await sleep(1000) |
There was a problem hiding this comment.
Is there any other signal we can wait for?
There was a problem hiding this comment.
I can't find any signal Redis emits when it starts up. One alternative would be to wait a smaller time (like 100 ms) in a loop, try to connect, and catch the failure. So it tries to connect every 100 ms. We could also have some maximum cutoff time (like 2-3 s).
There was a problem hiding this comment.
Ok, so this means leave it at 1 second?
There was a problem hiding this comment.
I meant taking the alternative approach you mentioned. Wait for a small amount of time, and check the status. Raise failure after a timeout.
There was a problem hiding this comment.
It turns out Redis has polling built in.
So I just removed the sleep, and disabled the "connection failed" error logging by default
No description provided.