fix: Resolve handful of CLI issues #1318
Conversation
orpheuslummis
added
bug
Codecov Report
@@ Coverage Diff @@
## develop #1318 +/- ##
===========================================
+ Coverage 70.53% 70.63% +0.10%
===========================================
Files 184 184
Lines 17786 17784 -2
===========================================
+ Hits 12545 12562 +17
+ Misses 4287 4270 -17
+ Partials 954 952 -2
|
| @@ -112,6 +114,12 @@ func (cfg *Config) LoadWithRootdir(withRootdir bool) error { | |||
| return err | |||
| } | |||
|
|
|||
| // using absolute rootdir for robustness | |||
| cfg.Rootdir, err = filepath.Abs(cfg.Rootdir) | |||
There was a problem hiding this comment.
question: Have you tested that this all works on database restart? I.e. to make sure #1293 isnt re-introduced
There was a problem hiding this comment.
I confirm that it works on database restart
orpheuslummis
force-pushed
the
orpheus/fix/cli-varia
branch
from
7df76b6 to
afbd42d
Compare
|
|
||
| datastore: | ||
| # Store can be badger | memory | ||
| # badger: fast pure Go key-value store optimized for SSDs (https://github.com/dgraph-io/badger) | ||
| # memory: in-memory version of badger | ||
| store: {{ .Datastore.Store }} | ||
| badger: | ||
| # The path to the database data file(s), relative paths here will be converted to absolute file paths | ||
| # on database start. | ||
| # The path to the database data file(s). |
There was a problem hiding this comment.
suggestion: I do think it is worth documenting that we will mutate the config file and replace relative paths with absolute paths. I see that as potentially surprising behaviour, and a behaviour that may potentially mess with CI systems (e.g. config files are sometimes directly pulled from source control, and impromptu edits can mess with the next pull)
There was a problem hiding this comment.
Currently we don't overwrite the config file. We write a config file when it doesn't exist. For now, the paths written are absolute paths, although what would be nicer is to have relative paths there, but which will probabIy be enabled by a refactor of config.
There was a problem hiding this comment.
for future reference - we chatted over discord, and I was wrong here - it doesnt mutate existing files :)
Resolves sourcenetwork#1321 Resolves sourcenetwork#1319 Resolves sourcenetwork#1317 Resolves sourcenetwork#1316 Resolves sourcenetwork#1315
Relevant issue(s)
Resolves #1321
Resolves #1319
Resolves #1317
Resolves #1316
Resolves #1315
Description
See resolved issues.
Tasks
How has this been tested?
Manually