727660 - mozmill doesn't work with HTTP_PROXY

Status: RESOLVED FIXED

(Testing Graveyard :: Mozmill, defect)

Description

[Dave Lam]

I'm running mozmill behind a firewall and I would like to send the report to the dashboard via CLI:
--report=http://mozmill-crowd.blargon7.com/db/

But mozmill complains connection time out(Errno: 145). Setting HTTP_PROXY doesn't work either while mozmill-automation/download.py works with HTTP_PROXY.

Comment 1

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Looks like it is because of httplib:
https://github.com/mozautomation/mozmill/blob/hotfix-1.5/mozmill/mozmill/__init__.py#L457

Jeff, why have we changed to httplib a while back? Why can't we simply use urllib which makes the proxy code transparent to the client code?

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+1]]

What I forgot to mention, our download script is using urllib so it works out of the box with proxies defined.

Comment 3

[Jeff Hammel]

We went from httplib2 to httplib because httplib2 is not part of the standard library, though both urllib and urllib2 are.  I usually use urllib2, though tbh i am not sure how either of them deal with proxies.

Comment 4

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Not sure about a POST request but retrieving data via GET will work that way:
http://hg.mozilla.org/qa/mozmill-automation/file/255935e8d585/libs/scraper.py#l219

Comment 5

[David Guo [:davidg]]

Attachment: Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/14

Pointer to Github pull-request

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 612693 [details]
Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/14

Patch looks fine. Beside that I want to have an additional review from Jeff.

Comment 7

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patched Mozmill 1.5.9

Dave, would you mind having a test with this patched version of Mozmill? It has the patch from this bug included and it should work with your proxy settings via ENV variables now. Would be great if you can give us feedback on it as soon as possible. Thanks!

Comment 8

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Clint, given that this is blocking a final 1.5.x release, it should also block Mozmill 2.0. If you don't think so please update the flag.

David, we would need a follow-up patch for the master/hotfix-2.0 branch. The code there is on another location so the patch wouldn't apply. Would be great if you can come up with a patch today.

Comment 9

[Dave Lam]

(In reply to Henrik Skupin (:whimboo) from comment #7)
> Dave, would you mind having a test with this patched version of Mozmill? It
> has the patch from this bug included and it should work with your proxy
> settings via ENV variables now. Would be great if you can give us feedback
> on it as soon as possible. Thanks!

I got another error(I'm running mozmill on solaris):
Sending results to 'http://mozmill-crowd.blargon7.com/db/' failed (HTTP Error 403: Forbidden).

Comment 10

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Dave, would you mind to join our #automation channel on IRC (irc.mozilla.org) so we can debug this further? It would help a lot here. Thanks.

Comment 11

[Jeff Hammel]

Comment on attachment 612693 [details]
Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/14

looks good to me

Comment 12

[Jeff Hammel]

We'll need this on all branches, I think

Comment 13

[Henrik Skupin [:whimboo][⌚️UTC+1]]

This patch shouldn't have been merged yet given the above mentioned feedback from Dave. So we will hold of from landing on other branches for now until the remaining issue has been solved.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Example script

Backed out the patch on hotfix-1.5:
https://github.com/mozautomation/mozmill/commit/9c1112f9d9632f5d63379e628108181c0b4b63ef

I had to backout the already landed patch because it causes a regression and we do no longer see the real error message. Once we were able to fix that we probably know what's wrong for Dave.

Attached you can find an example Python script which uses the old and the new code. Both should report the same information but don't:

$ python test.py 
{"error":"forbidden","reason":"This document requires the field application_id"}

Traceback (most recent call last):
  File "test.py", line 27, in <module>
    new()
  File "test.py", line 12, in new
    f = urllib2.urlopen(req)
  File "/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/urllib2.py", line 126, in urlopen
    return _opener.open(url, data, timeout)
  File "/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/urllib2.py", line 397, in open
    response = meth(req, response)
  File "/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/urllib2.py", line 510, in http_response
    'http', request, response, code, msg, hdrs)
  File "/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/urllib2.py", line 435, in error
    return self._call_chain(*args)
  File "/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/urllib2.py", line 369, in _call_chain
    result = func(*args)
  File "/System/Library/Frameworks/Python.framework/Versions/2.6/lib/python2.6/urllib2.py", line 518, in http_error_default
    raise HTTPError(req.get_full_url(), code, msg, hdrs, fp)
urllib2.HTTPError: HTTP Error 403: Forbidden

Comment 15

[David Guo [:davidg]]

I can't reproduce this on Ubuntu or OS X, but I'm currently looking into Solaris.

I have a hunch that the 403 error is actually coming from the recent dashboard validation changes. To post onto the dashboard requires the following keys in your JSON (https://github.com/whimboo/mozmill-dashboard/blob/master/dashboard.js#L36-45).

Dave, it would help guide me in the right direction if you can verify that your JSON blob has all the required keys.

You can log the JSON locally by using the following options:

--report=stdout [FirefoxBinary] > [output.txt]

If you're unsure, you can attach the output file and I'll be glad to help.

Comment 16

[David Guo [:davidg]]

Attachment: Example script v2

Comment 17

[David Guo [:davidg]]

I have tweaked the example script to mock a real report, and have it working with http_proxy on Solaris 11.

Comment 18

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Example script v3

Stripped down version of the example script which only includes the necessary parts and some more but not a whole report. David, please remove one of the required fields and try to upload. Check the reported failure for the old and new method. This is a regression we have to solve.

Comment 19

[David Guo [:davidg]]

Attachment: Patch for hotfix-1.5 [checked-in]

Pointer to Github pull-request

Comment 20

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Attachment: Patched Mozmill 1.5.9 v2

Dave, I have updated the patched version of Mozmill with the latest changes. Can you please check again and try to send a report. I'm kindly interested to see what's wrong on your side.

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+1]]

I talked with Dave yesterday and everything is correct. He was simply using mozmill directly instead of our automation scripts. So that part works fine.

David, have you had a chance to update the patch as requested in the pull?

Comment 22

[David Guo [:davidg]]

Hey Jeff, just wanted your feedback on how the exception is currently handled and if you would change it.

Henrik and I discussed alternatives here: http://pastebin.mozilla.org/1567282 .

I believe the bug has been resolved and I'll make a patch for 2.0 following your feedback.

Thanks.

Comment 23

[Jeff Hammel]

Comment on attachment 614298 [details]
Patch for hotfix-1.5 [checked-in]

I am not a big fan of 2., but 1. and 3. look fine

Comment 24

[David Guo [:davidg]]

Attachment: Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/20

Pointer to Github pull-request

Comment 25

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 614298 [details]
Patch for hotfix-1.5 [checked-in]

Landed as:
https://github.com/mozautomation/mozmill/commit/4ffc3da38f9e28faf6c5e01c6198ed2e1102e108

Comment 26

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Setting in-testsuite flag to indicate that we want to have a test for it once mozHTTPd will be used by Mozmill. For now it seems to be hard to test for sending reports.

Comment 27

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Comment on attachment 615397 [details]
Pointer to Github pull request: https://github.com/mozautomation/mozmill/pull/20

Adding missing review flag, so that we can get this finally in.

Comment 28

[Henrik Skupin [:whimboo][⌚️UTC+1]]

Landed on master:
https://github.com/mozautomation/mozmill/compare/74a9fa9...412e4ec

Thanks David!