Restructure venus temp - Githubissues

LinkedInAttic / venus.js

where bugs go to die

http://venusjs.readthedocs.io/en/latest/

Other

298 stars 44 forks source link

Restructure venus temp #350

Closed geoffberger closed 9 years ago

geoffberger commented 9 years ago

Here's a brief overview of what's happening:

will delete the temp directory prior to every run to make sure it doesn't get too large and no accidental timeouts occur
breaks down each temp directory by the port number - therefore in the home directory you would see .venus_temp-2013, .venus_temp-2014, and so forth
wrote helpers to aid in referencing temp directory across different files
switched promise order to make sure static directory is written at the right time (also rewrote it to remove nested callbacks)
wrote tests to support all these changes
added logging for new changes

sethmcl commented 9 years ago

What if we used the system temp directory and created a new folder for each run of venus, for example:

/tmp/venus # top level venus temp folder
/tmp/venus/run-[GUID] # run 1
/tmp/venus/run-[GUID] # run 2 ... etc

We could also add a venus clean command or something to wipe that out. Just a thought :) Thanks for the contributions!

Seth

jasonbelmonti commented 9 years ago

I think @sethmcl suggestion makes a lot of sense!

geoffberger commented 9 years ago

Thanks @sethmcl! It's funny, I was initially thinking the same when it came to grouping them together. Well, the naming wasn't the same, but more the directory hierarchy. The one thing I didn't group by was the GUID, which I like.

A question about that, when you say GUID, can you clarify that? Specifically are you referring to generating a GUID, getting the unique process id, or something else?

Updating the logic to do this shouldn't be too difficult. There is already a good amount of changes in this one pull request. Therefore, I'd lean towards doing the venus clean work in another PR once this has been merged into 2.x. Thanks again!

sethmcl commented 9 years ago

I agree with saving venus clean for a future PR. Can we just mention in the docs somewhere this new location of the temp files?

For the guid, I was thinking we could just use a library like https://www.npmjs.com/package/node-uuid

geoffberger commented 9 years ago

That sounds good regarding setting the guid. I got the structure in place for this so it'll be really easy to get that going. I'll add the guid, update the tests, and docs like you suggested within the next day or so.

I hope it's not an issue, but when I saw how easy it was to add the clean command, I ended up throwing it together really quickly. I need to add some tests around it, but that should be straight forward. I'll let you know when this is good to go. Thanks!

geoffberger commented 9 years ago

The squashed version of this can be seen here: https://github.com/linkedin/venus.js/pull/355