android talos doesn't have a proxy server



8 years ago
7 years ago


(Reporter: jmaher, Assigned: jmaher)



Firefox Tracking Flags

(Not tracked)


(Whiteboard: [mobile_unittests][android_tier_1])


(1 attachment, 2 obsolete attachments)

it appears that we were not running a network http proxy while running talos for android/tegra.
tested locally, would like to see in staging if we agree on this patch.
Assignee: nobody → jmaher
Attachment #562780 - Flags: review?(anodelman)
Split seems pretty fragile for this use, I think that you should look into urlparse for something that will provide better error messaging.
urlparse is helpful, but there are a lot of corner cases with it....let me see if I can find something better.  urlparse also doesn't split out the host:port, hmm.
My reading of the doc ( suggests that it understands port numbers in urls.  Maybe I just haven't hit the right corner case yet. :)
updated with urlparse.
Attachment #562780 - Attachment is obsolete: true
Attachment #562780 - Flags: review?(anodelman)
Attachment #562852 - Flags: review?(anodelman)
Since you already know the failure state, let's get this wrapped in a try:

+        #NOTE: if server has a scheme, this will fail, as well as the user_pref below will.
well the error is more like this:
if (server.startswith('http') or server.startswith('chrome') or server.startswith('file') or ...):
  urlparse('http://' + server)

there is no try/except thrown, it is just that we could get into a corner case where the hostname is the original scheme (i.e. http:), so would you be fine adding a check in both places for all types of schemes in the server variable?

fwiw, we have assumed there is no scheme on the webserver since this was originally developed (Nov 2009) and have never had a problem.  I agree we should clean up the code correctly, it is just a lot of parameter checking.  I also cannot assume that we will run perfconfigurator+run_tests without editing the intermediate config file.
Blocks: 438871
Attachment #562852 - Attachment is obsolete: true
Attachment #562852 - Flags: review?(anodelman)
Attachment #564841 - Flags: review?(wlachance)
Comment on attachment 564841 [details] [diff] [review]
add network proxy to remote.config (3.0)

Review of attachment 564841 [details] [diff] [review]:

Looks great.
Attachment #564841 - Flags: review?(wlachance) → review+
Closed: 8 years ago
Resolution: --- → FIXED
Whiteboard: [orange][mobile_unittests][android_tier_1] → [mobile_unittests][android_tier_1]
You need to log in before you can comment on or make changes to this bug.