search

joncrlsn / dque

dque is a fast, embedded, durable queue for Go
MIT License
767 stars 46 forks source link

Error when directory doesn't exist #7

Closed kbariotis closed 5 years ago

kbariotis commented 5 years ago

Was wondering if we could swap os.Mkdir with os.MkdirAll to allow for the creation of parents directories. At the moment, I have a folder local and set dque to create a queue in directory local/queues and I would expect that folder to be created.

https://github.com/joncrlsn/dque/blob/v2/queue.go#L68-L80

Would love to submit a PR if you agree too.

Thank you

joncrlsn commented 5 years ago

Hi Kostas, So the "name" variable has a value of "local/queues"? I can't see a problem with switching it to os.MkdirAll. Sure, submit a PR.

kbariotis commented 5 years ago

Not the queue name but the directory, and because of the condition in line 68, it throws an error. Great, will do. Thank you

joncrlsn commented 5 years ago

Oh, so that's different. The method was written assuming that the dirPath directory exists. I'd prefer if you make sure "local/queues" exists (and create it if it doesn't) before you call that method.

It was written that way so that if someone accidentally calls that method with an inappropriate or invalid dirPath value, they will get an error instead of having it create a nonsensical directory that they didn't want, and then continue on as if nothing is wrong. I understand the thinking behind utilities that try to minimize errors thrown, but I prefer the approach which lets the user know if things are not quite as expected.

kbariotis commented 5 years ago

Got it @joncrlsn , makes sense. Thank you again! 🙂