Skip to content
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

introduce "repo" package to manage IPFS_DIR concerns #376

Closed
3 tasks
btc opened this issue Nov 23, 2014 · 4 comments · Fixed by #578
Closed
3 tasks

introduce "repo" package to manage IPFS_DIR concerns #376

btc opened this issue Nov 23, 2014 · 4 comments · Fixed by #578
Labels
kind/enhancement A net-new feature or improvement to an existing feature kind/support A question or request for support
Milestone

Comments

@btc
Copy link
Contributor

btc commented Nov 23, 2014

issue

Introducing daemon --init broke eventlog configuration. Here's why:

  1. Now, users may run daemon without an initialized repo.
  2. the configureLogger hook checks to see whether the command usesConfig and usesRepo. If both are true, the logger is configured to use the IPFS_DIR. This is necessary since the logger logs to a file.

Since the daemon doesn't require a repo/config and configureLogger only runs if the upcoming command uses the config/repo, logs are not configured before running the daemon.

Notice that the one actual requirement for the hook is the existence of a config/repo.

Therefore, we should just check this property directly.

proposal

I propose we introduce a repo package to handle IPFS_DIR concerns. This would include the concern described in #358 (locking the repo).

Initially the package would have these exported functions:

  • func IsInitialized bool
  • func IsLocked bool
  • func Lock *Lock

Also...

We've also discussed centralizing option precedence. ie. the logic that determines whether to use environment variables, user-provided CLI arguments, or config file values to determine the value of an option (eg. debug). This may be out of the scope of this issue.

@btc btc added kind/enhancement A net-new feature or improvement to an existing feature kind/support A question or request for support labels Nov 23, 2014
@jbenet
Copy link
Member

jbenet commented Nov 24, 2014

👍 Ahh great, we're on the same page. #380 (comment)

@btc
Copy link
Contributor Author

btc commented Nov 24, 2014

We could even move the entire config into that

👍

probably the level db constructor as well. that way, config, datastore, and logs all live in repo.

and have core.IpfsNode hold a repo object

👍

Future work: Would be nice to also get rid of the online bool parameter and instead pass in a Transport. I came across an issue wherein you were discussing a simpilfied core API. This sort of fits into that broader discussion. This repo work can be done independently, but we may benefit from keeping the other concern in mind.

(repo instance) Or something?

👍

It becomes a lot easier to manage cores with an instance-scoped repo (rather than package scoped)

What if we had in-memory only repos? or repos not on the filesystem?

👍

very useful for programmatic use-cases

And we could move a lot of the config read/writing into the repo implementations. thoughts?

👍

yup

@btc btc self-assigned this Dec 9, 2014
@btc btc added this to the α milestone Dec 9, 2014
@btc btc mentioned this issue Jan 13, 2015
@btc btc added the status/in-progress In progress label Jan 13, 2015
@jbenet
Copy link
Member

jbenet commented Jan 15, 2015

@briantigerchow is this ready to close? or things remaining?

@btc
Copy link
Contributor Author

btc commented Jan 15, 2015

#578 closes this

@btc btc closed this as completed in #578 Jan 18, 2015
@btc btc removed the status/in-progress In progress label Jan 18, 2015
@btc btc removed their assignment Mar 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or improvement to an existing feature kind/support A question or request for support
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants