keep brim folders inside app data #714
Conversation
| @@ -51,16 +51,23 @@ const validateInput = (paths) => ({ | |||
|
|
|||
| const createDir = () => ({ | |||
| async do({dataDir}) { | |||
| await fsExtra.ensureDir(dataDir) | |||
| dataDir && (await fsExtra.ensureDir(dataDir)) | |||
There was a problem hiding this comment.
It seems like this whole step will be removed once #676 is done. All we might do is check that the user specified directory exists.
There was a problem hiding this comment.
Wait, I might understand the data_dir thing correctly. This might be fine.
| if (params.dataDir) { | ||
| createParams = {data_dir: params.dataDir} | ||
| } else { | ||
| createParams = {name: params.name} |
There was a problem hiding this comment.
Is there a reason why we don't send the name in all cases?
There was a problem hiding this comment.
I opted to send one or the other bc zqd only handles for one (https://github.com/brimsec/zq/blob/master/zqd/space/space.go#L108-L123) (giving preference to name by handling it first if it is present) and I thought it might confuse anyone looking at this code to see that it sends both but only uses name.
|
After testing this, I see some limitations here. After opening one pcap...
In the first situation it seems we ought open that space for them. The second situation seems like a new bug. I think we'd need to do this ZQD issue first brimdata/super#686 (generate space ids), before we'll be able to merge this PR without introducing bugs. Does that sound right? |
|
@jameskerr I'm inclined to agree. If we make the space id change first in the backend then I'm p sure this will work as is, which includes continuing to be able to make duplicate spaces if a user wants (though it may be better to not allow that and instead detect the duplicate on the FE and take the user direct to the existing space, as you suggest) |
Signed-off-by: Mason Fish <mason@looky.cloud>
Signed-off-by: Mason Fish <mason@looky.cloud>
mason-fish
force-pushed
the
673-use_default_brim_location
branch
from
b0e5bf4 to
1218ac9
Compare
Signed-off-by: Mason Fish <mason@looky.cloud>
| @@ -56,6 +56,7 @@ function getCurrentSpaceName(spaces, desired) { | |||
| function setSpace(dispatch, data, clusterId) { | |||
| globalDispatch(Spaces.setDetail(clusterId, data)) | |||
| dispatch(Search.setSpace(data.name)) | |||
| data = brim.interop.spacePayloadToSpace(data) | |||
There was a problem hiding this comment.
@jameskerr The problem is that the span parsing we are doing rn is late in the stack, in the reducer (https://github.com/brimsec/brim/pull/715/files#diff-31bf947bfb654ba8ccfd57214bfb417dR21), and yet the original span which has not been put through that adapter/parser continues to get passed along by this function to the rest of the initSpace chain. So I've adjusted this to do the same operation on a different copy and return that. Not a great pattern, would be better if we can handle this mutation inside the client, or at least much earlier before it gets passed around in different formats.
fixes #673
Our spaces are still named
.brimbut the internal folder we use is now stored within the app's data. I tested this in dev and also release.