[nv16] flexible bundle loading #8666
Conversation
There was a problem hiding this comment.
Having to modify your bundle.toml to specify an envvar which in turn is going to override any other setting seems like a circuitous way of doing things. It also defeats the one of the purposes, which is being able to check out the Lotus repo as-is (or download a pre-built binary) and make it use locally built bundles instead without any changes in sources.
| @@ -6,7 +6,7 @@ The bundles are specified in build/bundles.toml using the following syntax: | |||
| ```toml | |||
| [[bundles]] | |||
| version = X # actors version | |||
| release = tag # release gag | |||
| release = tag # release tag | |||
There was a problem hiding this comment.
Can we add examples of how to use path / envvar / url here
node/bundle/manifest.go
Outdated
| // this is a local bundle, specified by an env var to load from the filesystem | ||
| path := os.Getenv(bd.EnvVar) | ||
| if path == "" { | ||
| return xerrors.Errorf("bundle envvar is empty: %s", bd.EnvVar) |
There was a problem hiding this comment.
We could just fallthrough in this case, but if the EnvVar field is set, but there's no value provided user is probably just doing something wrong. Up to you.
|
ok fine. |
There was a problem hiding this comment.
Having to modify your bundle.toml to specify an envvar which in turn is going to override any other setting seems like a circuitous way of doing things. It also defeats the one of the purposes, which is being able to check out the Lotus repo as-is (or download a pre-built binary) and make it use locally built bundles instead without any changes in sources.
build/bundle.go
Outdated
| Path map[string]string | ||
| // URL is the (optional) bundle URL; takes precdence over github release | ||
| URL string | ||
| // CHecksum is the bundle sha256 checksume in hex, when specifying a URL. | ||
| Checksum string | ||
| URL map[string]BundleURL |
There was a problem hiding this comment.
I'd pluralize these field names.
There was a problem hiding this comment.
maybe, I kind of prefer singular here; but i don't feel strongly either way.
|
Yeah, let's get to its final "works really well" phase, and we can do some cleanup. |
|
@raulk Strong agree, there is definitely a round of cleanup that needs to happen -- ultimately these bundles are gonna be "never changing" things (except for devs and network upgrades), so something much more static will work. |
On top of #8658.
Adds flexibility for bundle fetching, which can use an env var or explicit URL to avoid downloading from github.
cc @travisperson