Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restructure venus temp #350

Closed

Conversation

geoffberger
Copy link

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
Copy link
Contributor

sethmcl commented Mar 7, 2015

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
Copy link
Contributor

I think @sethmcl suggestion makes a lot of sense!

@geoffberger
Copy link
Author

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
Copy link
Contributor

sethmcl commented Mar 12, 2015

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
Copy link
Author

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
Copy link
Author

The squashed version of this can be seen here: #355

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants