-
Notifications
You must be signed in to change notification settings - Fork 66
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
chore(deps): bump ipfs to v0.5.0 #1273
Conversation
looks like we're now clear of the GOPROXY problems of old.
@@ -129,7 +131,13 @@ func NewCAFSStore(ctx context.Context, cfg *config.Config) (store cafs.Filestore | |||
}, | |||
ipfs.OptsFromMap(cfg.Store.Options), | |||
} | |||
return ipfs.NewFilestore(fsOpts...) | |||
store, err = ipfs.NewFilestore(fsOpts...) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Won't let me comment on the actual import, but the change in cmd from importing qfs/cafs/ipfs
as qipfs
makes understanding what's happening much easier.
Recommend changing it in repo/buildrepo/build.go
as well! Perhaps also qipfs_http
} | ||
|
||
if err = f.Init(); err != nil { | ||
return err | ||
if errors.Is(err, qipfs.ErrNeedMigration) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to migrate before this point, I am getting a nil pointer dereference
error before we even get into the connect command code:
In our cmd PersistentPreRun
func, we call opt.Instance()
to get an instance so we can get to the config. opt.Instance()
runs o.Init()
, which calls lib.NewInstance
, which errors with the ipfs ErrNeedsMigration
error. o.Init()
returns the error, and opt.Instance()
does not display the error, but does pass back nil
as the instance pointer. When we try to access the config using this nil instance pointer, we get the nil pointer dereference
error.
Made a change to move the migration into o.Init()
, handling the confirmation there. But this also feels wrong, and seems like it should happen when we try to give the Instance the Filestore
, but still feeling for what is appropriate.
However, making this change meant I successfully migrated my repo!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
made these changes on branch chore_ipfs_bump_ramfox
, didn't want to push to this branch before knowing if these changes made sense
closing! We've merged a migration to ipfs v0.6.0 into master |
closes #1225
I'm going to leave this as a draft while we work on the 0.5.0 upgrade process. We'll only merge this after 0.5.0 has landed.