Skip to content

Add base-path option to the codechin cli arguments #1499

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 9, 2019

Conversation

HoOngEe
Copy link
Contributor

@HoOngEe HoOngEe commented Apr 26, 2019

Now It's possible to specify base-path on which the keys and db directory will be created when running codechain. But there already are options db-path and keys-path.
The rule relating to db-path, keys-path and base-path is like following:

db-path and keys-path have higher priority than base-path so if
both db-path and keys-path are set, base-path becomes meaningless.

it closes #236

@HoOngEe HoOngEe force-pushed the feature/specify-base-dir branch from 32bb133 to 3c283f5 Compare May 2, 2019 03:24
chain = "mainnet"


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Without this change, the code in the line https://github.com/CodeChain-io/codechain/pull/1499/files/3c283f526dfb43ca1261fa5a77f090e2626b41c7#diff-22a0b3dcef5d27fab89655ebdb330967R257 could be a cause of panic.
Do I need to remove this change and use unwrap_or for the line mentioned above?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does it panic when you remove an empty line?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I'm sorry I misunderstood that you mentioned about the +base_path = "." change.

@sgkim126
Copy link
Contributor

sgkim126 commented May 7, 2019

Hi @majecty @foriequal0,

I have a question about the behavior of a relative path.
What do you expect when you specify base-dir and specify db-path as a relative path?
A relative path from the current working directory? or from the base dir?

@majecty
Copy link
Contributor

majecty commented May 7, 2019

I'm not familiar with the setting two paths in CLI.
I found that in the Parity's config, the DB path is relative to the base path.
https://github.com/paritytech/parity-ethereum/blob/7c335e87642b848f61ab3ee7bbff5964b53d6f11/util/dir/src/lib.rs#L179

@HoOngEe HoOngEe force-pushed the feature/specify-base-dir branch from 3c283f5 to eebaf41 Compare May 7, 2019 10:33
Copy link
Contributor

@sgkim126 sgkim126 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's better to merge it for now and implement a sandbox correctly in the future.
Let's merge it.

@mergify mergify bot merged commit ba7df39 into CodeChain-io:master May 9, 2019
@HoOngEe HoOngEe deleted the feature/specify-base-dir branch May 9, 2019 02:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add option to specify base directory
3 participants