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

fix(eventlog) initialization #380

Merged
merged 3 commits into from
Nov 25, 2014
Merged

fix(eventlog) initialization #380

merged 3 commits into from
Nov 25, 2014

Conversation

btc
Copy link
Contributor

@btc btc commented Nov 24, 2014

This PR fixes two issues.

daemon --init breaks eventlog configuration

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 may want to simply check this property directly. Thoughts?

runtime logging system is not configured upon initialization

Now, after initialization, we apply the config.Log settings to the logging system. This ensures logging works when the daemon is started using daemon --init

Third, this paves the way for the repo package described in #376.

@btc btc added status/in-progress In progress codereview and removed status/in-progress In progress labels Nov 24, 2014
@jbenet
Copy link
Member

jbenet commented Nov 24, 2014

LGTM! 👍 I like the repo package idea. We could even move the entire config into that, and have core.IpfsNode hold a repo object:

type Repo struct {
  Path string
  Config config.Config
  Datastore ds.Datastore
  ...
}

// move the file lock stuff here.
func (r *Repo) Lock() {}
func (r *Repo) Unlock() {}

Or something?

Also, maybe Repo should be an interface. What if we had in-memory only repos? or repos not on the filesystem?

type Repo interface {
  Config() config.Config
  Datastore() ds.Datastore
  ...
}

type fsRepo struct {
  path string
  ...
}

func NewFSRepo(path string) (Repo, error) { ... }

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

@btc
Copy link
Contributor Author

btc commented Nov 24, 2014

RFM

re repo: hell yes (replied in #376)

Brian Tiger Chow added 3 commits November 24, 2014 16:05
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>

Conflicts:
	cmd/ipfs/main.go
This commit applies the logging settings to the logging system upon
initialization `ipfs init` and `ipfs daemon --init`.

License: MIT
Signed-off-by: Brian Tiger Chow <brian@perfmode.com>
@btc btc force-pushed the fix/eventlog-init-condition branch from 6c73330 to 90ed148 Compare November 25, 2014 00:06
btc pushed a commit that referenced this pull request Nov 25, 2014
@btc btc merged commit ca3e830 into master Nov 25, 2014
@btc btc removed the codereview label Nov 25, 2014
@btc btc deleted the fix/eventlog-init-condition branch November 25, 2014 00:14
ariescodescream pushed a commit to ariescodescream/go-ipfs that referenced this pull request Oct 23, 2021
Update tests to use Ed25519 when acceptable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants