fs-repo-11-to-12: add env vars for configuring the number of workers and the sync size #149
Conversation
…r of workers and the sync size
| workerEnvVar := "IPFS_FS_MIGRATION_11_TO_12_NWORKERS" | ||
| syncSizeEnvVar := "IPFS_FS_MIGRATION_11_TO_12_SYNC_SIZE" |
There was a problem hiding this comment.
Figured long names would prevent conflict and almost nobody should need to set these, but if you have other names in mind that's fine by me.
| if err != nil { | ||
| panic(err) | ||
| } | ||
| if nworkers < 1 { | ||
| panic("number of workers must be at least 1") | ||
| } |
There was a problem hiding this comment.
We could log an error and fallback to the defaults instead of panicking here, but this seems reasonable to let the user know they did something wrong.
| if syncSize < 1 { | ||
| panic("sync size bytes must be at least 1") | ||
| } |
There was a problem hiding this comment.
Should we set this higher (e.g. 1MB minimum) just so people don't accidentally think 1 == 1MB or something?
Similarly is there any use for setting 0 to mean only syncing at the end of the migration?
There was a problem hiding this comment.
let's rename the env var to sync_size_bytes?
| @@ -4,6 +4,8 @@ test_description="Simple fs-repo-migrations tests" | |||
|
|
|||
| . lib/test-lib.sh | |||
|
|
|||
| latestRepoVersion="12" | |||
There was a problem hiding this comment.
This seems like a wacky change to have to do every time we do a release. Probably we could/should just hard code an immutable distpath so that the repo version is fixed, but at the moment we're just trying to get CI to be green.
Avoid that there is confusion about the unit used.
|
And I'm not sure why tests were failing |
|
@hsanjuan any learned lessons or recommendations for if/how these variables should be set for us to add to the release notes? |
|
No description provided.