Fix default path of chain:export command #1035
Conversation
Kupuyc
changed the title
|
Checked this out a bit, I'm still seeing the issue. Testing on a mac,
Thinking about this a bit more, this might be difficult to get right with the way it's currently coded, because we use a slightly different file structure in Docker, brew, built from source, etc. |
|
@mat-if Interesting... Thank you for information. I will check. |
|
I had an idea last night. The easiest way to fix this is probably to store the json in the datadir. We would probably need to change the default path on the ironfish-graph-explorer as well to look in the datadir, as well, and give the file a more specific name, like 'chain-export.json' or something. |
|
Yeah... probably it is the best way. Let's ask team members in discord next monday. |
| default: (): Promise<string> => { | ||
| Assert.isNotUndefined(process.env.PWD) | ||
|
|
||
| return Promise.resolve(`${process.env.PWD}/../ironfish-graph-explorer/src/data.json`) |
There was a problem hiding this comment.
This PR still uses incorrect relative paths. How about we just set the default path to the data directory in a folder called exports? I would recommend maybe just removing this default entirely, then below use the data dir from this.sdk.config.dataDir where the data dir is stored if no path is specified.
There was a problem hiding this comment.
It's done by second way: default value of flag set to empty string and we will resolve path using this.sdk.config.dataDir in that case.
| @@ -18,7 +18,7 @@ export default class Export extends IronfishCommand { | |||
| char: 'p', | |||
| parse: (input: string): Promise<string> => Promise.resolve(input.trim()), | |||
| required: false, | |||
| default: '../ironfish-graph-explorer/src/data.json', | |||
| default: '', | |||
There was a problem hiding this comment.
We can just delete this now
| default: '', |
There was a problem hiding this comment.
Done.
Also, I added path accessibility verification.
NullSoldier
force-pushed
the
fixExportDefaultPath
branch
from
7008c0f
to
69feb5c
Compare
NullSoldier
force-pushed
the
fixExportDefaultPath
branch
from
69feb5c
to
256601b
Compare
|
I just removed the check access, and just had it recursively create the folder for better usability. It's now good to go. |
Summary
Fixes #997
Default path of
ironfish chain:exportcommand is constant and relative -../ironfish-graph-explorer/src/data.json. It means command will always fail except when called from~/ironfish/*subfolder (in situation when-pflag is not set). It possible to fix such behavior usingprocess.env.PWD.Testing Plan
There is no test for this command - it should be tested manually. Run
ironfish chain:exportw/o-pflag from different places.Breaking Change
Is this a breaking change? If yes, add notes below on why this is breaking and
what additional work is required, if any.