application-research / estuary

A custom IPFS/Filecoin node that makes it easy to pin IPFS content and make Filecoin deals.
https://docs.estuary.tech
Other
240 stars 66 forks source link

Need to manually create datadir for shuttles to not hit peer.key does not exist error #714

Open neelvirdy opened 1 year ago

neelvirdy commented 1 year ago

Try running a shuttle with a --datadir=

value that doesn't exist as a directory yet. You get a /peer.key does not exist error instead of it being created for you.

@snissn

snissn commented 1 year ago

this makes sense and i see that it's not just a pinqueue thing or a shuttle thing - when i run the main cmd: % ./estuary --datadir=./aiowejfokj 2022-12-08T16:45:47.316Z INFO estuary estuary/main.go:573 estuary version: v0.2.1 {"app_version": "v0.2.1"} 2022-12-08T16:45:47.334Z FATAL estuary estuary/main.go:797 could not run estuary app: open aiowejfokj/estuary-peer.key: no such file or directory {"app_version": "v0.2.1"}

two options for fix if we make a change:

  1. make the directory automatically ( can this be bad? maybe failing is better)
  2. make the error message clearer

reason to keep it the same, if you're normally starting a node wth ./estuary --datadir=/mnt/data and then you start wth a typo like ./estuary --datadir=/mnt/daata an error is better than creating a new folder

neelvirdy commented 1 year ago

If we were to combine this with https://github.com/application-research/estuary/issues/715, then we know what the default datadir is and can auto-create it if the default is being used. If the default is not being used, then i think it's a good point we should throw an error instead of auto-creating it if it doesn't already exist, but ideally communicate it in a clearer error message. You could also make the argument that they shouldn't be overriding this flag in any case, and it shouldn't be an option to override it if we did https://github.com/application-research/estuary/issues/715.

en0ma commented 1 year ago
  • make the directory automatically ( can this be bad? maybe failing is better)
data, err := ioutil.ReadFile(filepath.Clean(kf))
    if err != nil {
        if !os.IsNotExist(err) {
            return nil, err
        }

We already do this, I do not think datadir= and datadir=./ are valid relative dirs that's why both failed. We should probably validate the datadir before starting. I added a validation method on the config recently.