fixes bug 788085 - automate setup/run of socorro dev mode #1665
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
||
|
|
||
| def main(): | ||
| parser = argparse.ArgumentParser(prog='socorro') |
There was a problem hiding this comment.
I thought argparse only existed in Python2.7 and above? Did you use a backport?
There was a problem hiding this comment.
We already have the pypi version for 2.6 in our requirements file
|
@twobraids - reverted those logging changes |
|
After running vagrant up
vagrant ssh
cd src/socorro/
./bin/socorro setupI got that error: http://pastebin.mozilla.org/3608629 |
|
@AdrianGaudebert thanks, should make that output prettier - the error is: Do you have a google-breakpad/ directory that you built on a different OS or arch (e.g. 64 bit linux)?
|
|
@AdrianGaudebert to confirm this is the case, can you please try: If it still fails then the problem is elsewhere :) |
|
@rhelmer A new error occurred (I indeed had a It happened during |
|
@AdrianGaudebert hmm could you try removing "./exploitable" directory and then running I didn't realize how problematic sharing workspaces are between vagrant/mac :) if this is indeed the problem. |
|
@AdrianGaudebert actually perhaps we should document this, so people always get a clean install: What do you think? That calls |
| logger.addHandler(logging.StreamHandler()) | ||
|
|
||
|
|
||
| class Socorro(): |
There was a problem hiding this comment.
No reason not to use a new-style class
There was a problem hiding this comment.
I'm not really sure why all these routines are wrapped in a class, except to share the superuser bit of state. But that's actually a singleton in multiton's clothing: it's always the same for one process invocation (modulo people scribbling evilly on os.environ). If we really wanted to have an excuse to make this a class, we should pass os.environ['USER'] in when Socorro is instantiated.
There was a problem hiding this comment.
Well originally I had started doing this using configman, but had trouble figuring out how to get it to handle the subcommand style of parsing I am using argparse for. Currently the only state they share is that superuser string but I could see it being useful to share more.
I'll make the change you suggest, passing in that environment variable when Socorro is instantiated.
There was a problem hiding this comment.
Well originally I had started doing this using configman, but had trouble figuring out how to get it to handle the subcommand style
I've got examples for that, but I don't think it's worth going back to do it.
----- Original Message -----
@@ -0,0 +1,206 @@
+import argparse
+import logging
+import os
+import shutil
+import socket
+import subprocess
+import sys
+
+logger = logging.getLogger('socorro')
+logger.setLevel(logging.INFO)
+logger.addHandler(logging.StreamHandler())
+
+
+class Socorro():Well originally I had started doing this using configman, but had trouble
figuring out how to get it to handle the subcommand style of parsing I am
using argparse for. Currently the only state they share is that
superuserstring but I could see it being useful to share more.I'll make the change you suggest, passing in that environment variable when
Socorrois instantiated.
Reply to this email directly or view it on GitHub:
https://github.com/mozilla/socorro/pull/1665/files#r7740062
ghost
assigned
|
@AdrianGaudebert @twobraids @erikrose thanks for your review comments! Everything is addressed, anyone want to do a final once-over and r+ this for me ? :) I will probably hold off until after release to land it, since it touches quite a bit. I don't expect it to break anything we use for production but couldn't hurt. |
|
Could you please update the |
|
Tested this locally, but there doesn't seem to be a server installed to serve crash-stats. I don't have apache or httpd on the box, and I don't see it in puppet as a dep. |
|
Tried this again, now knowing that there's a |
|
@lonnen sent an email, but the Apache missing is expected I think, since this runs everything in dev mode. So django will be on port 8000 for instance. On the second one, how long did you wait? It takes quite a while to populate the virtualenv from scratch. If you run without Originally I had it rerun Oh also could you try |
|
I guess I didn't let it run long enough. It took 23 minutes. There was still an error, and it's difficult to parse what's going on. |
|
Ouch that's terrible sorry, I'll fix that up. Here's the actual error: |
|
@lonnen OK! I fixed the output and also the underlying problem here, mind trying again? You should be able to just do Do you think we should have some kind of progress or show output as the virtualenv is built, since it takes so long? |
|
Given how long it runs, the script should put out a warning or something to reassure the user it is running. The Boxen install takes a while, but keeps the user updated with What's the advantage of this python script over improving the Makefile? |
|
@lonnen having it print that this will take a while is a good idea, thanks! I am hoping to eventually replace the Makefile with this, in my opinion:
Make has some advantages for sure, it's been around a very long time and is more discoverable without looking at the docs at all (assuming running plain |
| then | ||
| echo "Removing virtualenvs" | ||
| rm -rf $VENV | ||
| rm -rf webapp-django/$VENV |
There was a problem hiding this comment.
after a partial ./bin/socorro setup I restarted it with --clean and encountered:
rm: cannot remove `webapp-django/localhost-socorro-virtualenv/bin': Directory not empty
so I'm not sure if the -rf flags are being passes appropriately.
|
|
I had that commit before this most recent attempt. I believe the PATH change from the line above does not propagate through the rest of the build target because each line in a Makefile is executed in its own shell. |
|
I've made some local mods to test this idea out. I'll get back to you in a moment. |
|
This is all complicated by intermittent npm failures this week. |
|
@lonnen ugh. So one thing I noticed digging into this is, bootstrap does more than we need for a dev install - |
|
Got it running, finally. A couple of notes:
|
|
@lonnen thanks for the thorough testing!
|
|
So the provisioned box is 529MB, with the base CentOS box being 435MB. It's probably worth packaging our own box (note that this yum issue isn't new, it's been there since we switched from Ubuntu to CentOS, but this will work around it and remove variability) |
|
Hrm so thinking about the documentation inconsistency issue - since we are only recommending vagrant for dev setups (which I think is appropriate), we might be better off just forwarding ports from vagrant to the host instead of giving the VM its own IP address. This would allow us to remove the whole |
|
After reflecting on this, I think we should step back and work on some of the setup steps before trying to automate them, it's still too flaky to work reliably even if it's all wrapped up. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.