1282841 - can't set background rate to 0 for new rules

Status: RESOLVED FIXED

(Release Engineering Graveyard :: Applications: Balrog (frontend), defect)

Description

[Justin Wood (:Callek)]

Attachment: balrog_error.png

Similar to Bug 1282838 I had initially tried to "Duplicate" the official Firefox rule, and got an error about being unable to set the "Rate" to 0%.

0% is a valid rate and shouldn't error.

Comment 1

[[e-mail] (:bhearsum)]

I think this happens with any sort newly added rule.

Comment 2

[Richa]

Is this bug unresolved? can i start working on it?

Comment 3

[Njira]

(In reply to Richa from comment #2)
> Is this bug unresolved? can i start working on it?

Hi Richa. I have definitely stayed with this bug longer than could be acceptable. I have reassigned it to you. Thanks so much for the help. Cheers

Comment 4

[Richa]

how am i supposed to proceed? this i my first bug.Can u guys please help?

Comment 5

[[e-mail] (:bhearsum)]

(In reply to Richa from comment #4)
> how am i supposed to proceed? this i my first bug.Can u guys please help?

To get started, I recommend installing Docker and Docker Compose, and making sure you can run Balrog locally with the instructions at https://wiki.mozilla.org/Balrog#Hacking. Once you've got that working, we can talk about the specifics of this bug.

Comment 6

[[e-mail] (:bhearsum)]

Looks like this isn't being actively worked on...I'm going to unassign it for now.

Comment 7

[Michael Parlato]

(In reply to Ben Hearsum (:bhearsum) from comment #5)

> To get started...

Hi Ben - I'd like to take this up as my first bug. I followed your advice from comment #5 above and installed Docker/Docker Compose. I'm in the midst of cloning and running Balrog now. If you have any advice for first steps once I get that finished, it would be much appreciated!

Comment 8

[[e-mail] (:bhearsum)]

(In reply to Michael Parlato from comment #7)
> (In reply to Ben Hearsum (:bhearsum) from comment #5)
> 
> > To get started...
> 
> Hi Ben - I'd like to take this up as my first bug. I followed your advice
> from comment #5 above and installed Docker/Docker Compose. I'm in the midst
> of cloning and running Balrog now. If you have any advice for first steps
> once I get that finished, it would be much appreciated!

Excellent, that's a great start! Here's what I'd suggest next:
1) Reproduce the error in the UI with the Developer Toolbar open, make note of the Request Headers & Body sent to server.
2) Write a unit test that can reproduce the problem. It will probably be very similar to testNewRulePostJSON from https://github.com/mozilla/balrog/blob/master/auslib/test/admin/views/test_rules.py#L28. You can run tests with "bash run-tests.sh".
3) Try to fix it! The most likely place for the fix will be https://github.com/mozilla/balrog/blob/master/auslib/admin/views/rules.py#L36 or https://github.com/mozilla/balrog/blob/master/auslib/admin/views/forms.py#L170.
4) Once you've managed to get your unit test passing, verify the fix in the UI as well, just to be sure.

If you have any questions or trouble with any of the above feel free to join us in IRC (irc://irc.mozilla.org/#balrog) to ask.

Comment 9

[Michael Parlato]

Thanks, Ben. I was able to reproduce the error and write a quick test, basically replicating the one you referenced. I hope it's alright if I bounce my initial thoughts off of you here...

It looks like the problem is with WTForms validator Required(). Others (http://stackoverflow.com/questions/25335398/wtforms-integerfield-skips-coercion-when-string-value-is-0) have said the issue is with IntegerField() failing to coerce str '0' to int 0. However, removing Required() and leaving NumberRange(0, 100) passes with 0 as an input, so that doesn't make sense.

My first thought is to remove Required() from Priority and Rate field validations and instead use only NumberRange() on both. That solution works fine for 0 input, but throws an internal server error when no input is present. That's the bad news. The good news is that Required() wasn't preventing that - it happens regardless. Here's the console log when no input is present for either Priority or Rate fields:

parent_controller.js:16 500 "API Proxying to `/api/rules` failed with: `Error: socket hang up`

Thoughts?

Comment 10

[[e-mail] (:bhearsum)]

(In reply to Michael Parlato from comment #9)
> Thanks, Ben. I was able to reproduce the error and write a quick test,
> basically replicating the one you referenced. I hope it's alright if I
> bounce my initial thoughts off of you here...
> 
> It looks like the problem is with WTForms validator Required(). Others
> (http://stackoverflow.com/questions/25335398/wtforms-integerfield-skips-
> coercion-when-string-value-is-0) have said the issue is with IntegerField()
> failing to coerce str '0' to int 0. However, removing Required() and leaving
> NumberRange(0, 100) passes with 0 as an input, so that doesn't make sense.
> 
> My first thought is to remove Required() from Priority and Rate field
> validations and instead use only NumberRange() on both. That solution works
> fine for 0 input, but throws an internal server error when no input is
> present. That's the bad news. The good news is that Required() wasn't
> preventing that - it happens regardless. Here's the console log when no
> input is present for either Priority or Rate fields:
> 
> parent_controller.js:16 500 "API Proxying to `/api/rules` failed with:
> `Error: socket hang up`
> 
> Thoughts?

Hm, I'm not able to reproduce a 500 with Required() present - maybe I'm misunderstanding what you said? I ran:
curl 'http://localhost:8080/api/rules' -H 'Host: localhost:8080' -H 'User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:50.0) Gecko/20100101 Firefox/50.0' -H 'Accept: application/json, text/plain, */*' -H 'Accept-Language: tl,en-CA;q=0.5' --compressed -H 'Content-Type: application/json;charset=utf-8' -H 'Referer: http://localhost:8080/rules' -H 'Content-Length: 543' -H 'DNT: 1' -H 'Connection: keep-alive' -d '{"alias":null,"buildID":null,"buildTarget":null,"channel":"release","comment":"No System Addons for <44.0. Partly to work around bug 1294395.","data_version":"","distVersion":null,"distribution":"default","fallbackMapping":null,"headerArchitecture":null,"locale":null,"mapping":"SystemAddons-no-update","osVersion":null,"priority":1005,"product":"SystemAddons","rule_id":"","systemCapabilities":null,"update_type":"minor","version":"<44.0","whitelist":null,"_duplicate":true,"csrf_token":"1478094779##488ee96c94b044ada074f845fd00daec954f382b"}'
And got:
{"backgroundRate": ["This field is required."]}

With this output in the backend:
balrogadmin_1  | [pid: 15|app: 0|req: 9/9] 172.17.0.5 () {56 vars in 950 bytes} [Wed Nov  2 12:52:59 2016] POST /api/rules => generated 47 bytes in 11 msecs (HTTP/1.1 400) 5 headers in 326 bytes (1 switches on core 0)
balrogadmin_1  | 2016-11-02 12:53:14,865 - DEBUG - PID: 15 - Request: 140545652603600 - RulesAPIView.post#61: processing POST request to /rules
balrogadmin_1  | 2016-11-02 12:53:14,866 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms.process_formdata#72: No value list, setting self.data to None
balrogadmin_1  | 2016-11-02 12:53:14,867 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms.process_formdata#72: No value list, setting self.data to None
balrogadmin_1  | 2016-11-02 12:53:14,867 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms.process_formdata#72: No value list, setting self.data to None
balrogadmin_1  | 2016-11-02 12:53:14,867 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms.process_formdata#72: No value list, setting self.data to None
balrogadmin_1  | 2016-11-02 12:53:14,867 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms.process_formdata#72: No value list, setting self.data to None
balrogadmin_1  | 2016-11-02 12:53:14,867 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms.process_formdata#72: No value list, setting self.data to None
balrogadmin_1  | 2016-11-02 12:53:14,867 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms.process_formdata#72: No value list, setting self.data to None
balrogadmin_1  | 2016-11-02 12:53:14,867 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms.process_formdata#72: No value list, setting self.data to None
balrogadmin_1  | 2016-11-02 12:53:14,868 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms.process_formdata#72: No value list, setting self.data to None
balrogadmin_1  | 2016-11-02 12:53:14,868 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms.process_formdata#72: No value list, setting self.data to None
balrogadmin_1  | 2016-11-02 12:53:14,870 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms._validator#109: starting in version_validator: field.data is <44.0
balrogadmin_1  | 2016-11-02 12:53:14,870 - DEBUG - PID: 15 - Request: 140545652603600 - auslib.admin.views.forms._validator#92: starting in operator_validator: field.data is None
balrogadmin_1  | 2016-11-02 12:53:14,870 - WARNING - PID: 15 - Request: 140545652603600 - RulesAPIView._post#44: Bad input: {'backgroundRate': [u'This field is required.']}


I certainly wouldn't be surprised if Required is working funny for values that are valid but evaluate to False, though. Maybe there's a well known solution for that? I'm a bit wary of dropping Required() from these fields, but if there's some way to do that and still have them throw proper 400s when the field is missing, that would be fine.

Comment 11

[Michael Parlato]

Maybe I'm doing something incorrectly:

curl 'http://1ccept-Encoding: gzip, deflate, br' -H 'Accept-Language: en-US,en;q=0.8' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36' -H 'Content-Type: application/json;charset=UTF-8' -H 'Accept: application/json, text/plain, */*' -H 'Referer: http://127.0.0.1:8080/rules' -H 'Cookie: session=eyJjc3JmX3Rva2VuIjp7IiBiIjoiWVRCbFlXSXhaV0l4TXpWaU1ERXdaRGs0WTJVMFlqbGhPR0ptTkRBNE5UWmtZV1U0TVRNeFpRPT0ifX0.Cvulxg.EJqhKXm6b5hjMbvKplfFonkJkuc' -H 'Connection: keep-alive' --data-binary '{"alias":null,"backgroundRate":null,"buildID":null,"buildTarget":null,"channel":"release-sysaddon","comment":"No System Addons for <44.0. Partly to work around bug 1294395.","data_version":"","distVersion":null,"distribution":"default","fallbackMapping":null,"headerArchitecture":null,"locale":null,"mapping":"SystemAddons-no-update","osVersion":null,"priority":1005,"product":"SystemAddons","rule_id":"","systemCapabilities":null,"update_type":"minor","version":"<44.0","whitelist":null,"_duplicate":true,"csrf_token":"1478107734##99e5c7f09fbc4294f6b8820da81ec3b240b2d4a8"}'

And the response:
API Proxying to `/api/rules` failed with: `Error: socket hang up`

This happens when I set either backgroundRate or priority to null. Let me know if you can replicate with the above - if not maybe I should clone again?

Comment 12

[[e-mail] (:bhearsum)]

(In reply to Michael Parlato from comment #11)
> Maybe I'm doing something incorrectly:
> 
> curl 'http://1ccept-Encoding: gzip, deflate, br' -H 'Accept-Language:
> en-US,en;q=0.8' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X
> 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71
> Safari/537.36' -H 'Content-Type: application/json;charset=UTF-8' -H 'Accept:
> application/json, text/plain, */*' -H 'Referer: http://127.0.0.1:8080/rules'
> -H 'Cookie:
> session=eyJjc3JmX3Rva2VuIjp7IiBiIjoiWVRCbFlXSXhaV0l4TXpWaU1ERXdaRGs0WTJVMFlqb
> GhPR0ptTkRBNE5UWmtZV1U0TVRNeFpRPT0ifX0.Cvulxg.EJqhKXm6b5hjMbvKplfFonkJkuc'
> -H 'Connection: keep-alive' --data-binary
> '{"alias":null,"backgroundRate":null,"buildID":null,"buildTarget":null,
> "channel":"release-sysaddon","comment":"No System Addons for <44.0. Partly
> to work around bug
> 1294395.","data_version":"","distVersion":null,"distribution":"default",
> "fallbackMapping":null,"headerArchitecture":null,"locale":null,"mapping":
> "SystemAddons-no-update","osVersion":null,"priority":1005,"product":
> "SystemAddons","rule_id":"","systemCapabilities":null,"update_type":"minor",
> "version":"<44.0","whitelist":null,"_duplicate":true,"csrf_token":
> "1478107734##99e5c7f09fbc4294f6b8820da81ec3b240b2d4a8"}'
> 
> And the response:
> API Proxying to `/api/rules` failed with: `Error: socket hang up`
> 
> This happens when I set either backgroundRate or priority to null. Let me
> know if you can replicate with the above - if not maybe I should clone again?

It looks your curl command is a bit mangled...I see "http://1ccept-Encoding" instead of "http://localhost" like mine. Or maybe your paste from the console back into Bugzilla got mangled?

Either way, the full output from the docker containers would be very helpful - it sounds like one of the containers may not have come up correctly.

It may also help to chat in real time on IRC. If you're able, I'm usually around from 1300-2100 UTC.

Comment 13

[Michael Parlato]

Ya it was mangled in transit - here's the actual:

curl 'http://localhost:8080/api/rules' -H 'Origin: http://localhost:8080' -H 'Accept-Encoding: gzip, deflate, br' -H 'Accept-Language: en-US,en;q=0.8' -H 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36' -H 'Content-Type: application/json;charset=UTF-8' -H 'Accept: application/json, text/plain, */*' -H 'Referer: http://localhost:8080/rules' -H 'Cookie: ajs_user_id=null; ajs_group_id=null; ajs_anonymous_id=%2281d82dcf-0c88-4ad9-8cce-c5dfc9a639fc%22; session=eyJjc3JmX3Rva2VuIjp7IiBiIjoiWkRGbU5HVmpORFl4T1dZd05qZ3pNRGd6T1dSaE9XWmhZVGN4WTJNeU4yTmpNVFkyTVdKa1pnPT0ifX0.Cvvj-A.KFq4ZpOOy2_RZrjcR_IEoAo4-qI' -H 'Connection: keep-alive' --data-binary '{"alias":null,"backgroundRate":null,"buildID":null,"buildTarget":null,"channel":"release-sysaddon","comment":"No System Addons for <44.0. Partly to work around bug 1294395.","data_version":"","distVersion":null,"distribution":"default","fallbackMapping":null,"headerArchitecture":null,"locale":null,"mapping":"SystemAddons-no-update","osVersion":null,"priority":1005,"product":"SystemAddons","rule_id":"","systemCapabilities":null,"update_type":"minor","version":"<44.0","whitelist":null,"_duplicate":true,"csrf_token":"1478123656##dc560859ba2a5a15f8cbd66eec0aed1ed9298e5e"}'

And again the response:
API Proxying to `/api/rules` failed with: `Error: socket hang up`

Sure we can try to chat tomorrow around that time.

Comment 14

[[e-mail] (:bhearsum)]

(In reply to Michael Parlato from comment #13)
> Ya it was mangled in transit - here's the actual:
> 
> curl 'http://localhost:8080/api/rules' -H 'Origin: http://localhost:8080' -H
> 'Accept-Encoding: gzip, deflate, br' -H 'Accept-Language: en-US,en;q=0.8' -H
> 'User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11_6)
> AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.71 Safari/537.36' -H
> 'Content-Type: application/json;charset=UTF-8' -H 'Accept: application/json,
> text/plain, */*' -H 'Referer: http://localhost:8080/rules' -H 'Cookie:
> ajs_user_id=null; ajs_group_id=null;
> ajs_anonymous_id=%2281d82dcf-0c88-4ad9-8cce-c5dfc9a639fc%22;
> session=eyJjc3JmX3Rva2VuIjp7IiBiIjoiWkRGbU5HVmpORFl4T1dZd05qZ3pNRGd6T1dSaE9XW
> mhZVGN4WTJNeU4yTmpNVFkyTVdKa1pnPT0ifX0.Cvvj-A.KFq4ZpOOy2_RZrjcR_IEoAo4-qI'
> -H 'Connection: keep-alive' --data-binary
> '{"alias":null,"backgroundRate":null,"buildID":null,"buildTarget":null,
> "channel":"release-sysaddon","comment":"No System Addons for <44.0. Partly
> to work around bug
> 1294395.","data_version":"","distVersion":null,"distribution":"default",
> "fallbackMapping":null,"headerArchitecture":null,"locale":null,"mapping":
> "SystemAddons-no-update","osVersion":null,"priority":1005,"product":
> "SystemAddons","rule_id":"","systemCapabilities":null,"update_type":"minor",
> "version":"<44.0","whitelist":null,"_duplicate":true,"csrf_token":
> "1478123656##dc560859ba2a5a15f8cbd66eec0aed1ed9298e5e"}'
> 
> And again the response:
> API Proxying to `/api/rules` failed with: `Error: socket hang up`
> 
> Sure we can try to chat tomorrow around that time.

Hm, it sounds like the "balrogadmin" container is not coming up correctly for you for some reason. Could you provide the full output of "docker-compose up" so we can have a look at that?

Comment 15

[[e-mail] (:bhearsum)]

I'm going to unassign this for now since there's been no activity. If you want to have another look, just let me. Sorry the bootstrapping process was so difficult :(.

Comment 16

[Tomislav Jurin]

If it is possible I would like to take a shoot at it :)

Comment 17

[[e-mail] (:bhearsum)]

(In reply to Tomislav Jurin from comment #16)
> If it is possible I would like to take a shoot at it :)

You're welcome to! Here's what I'd recommend as next steps:
1) Get Balrog up and running locally (instructions on https://github.com/mozilla/balrog)
2) Reproduce the error in the UI with the Developer Toolbar open, make note of the Request Headers & Body sent to server.
3) Write a unit test that can reproduce the problem. It will probably be very similar to testNewRulePostJSON from https://github.com/mozilla/balrog/blob/master/auslib/test/admin/views/test_rules.py#L28. You can run tests with "bash run-tests.sh".
4) Try to fix it! The most likely place for the fix will be https://github.com/mozilla/balrog/blob/master/auslib/admin/views/rules.py#L36 or https://github.com/mozilla/balrog/blob/master/auslib/admin/views/forms.py#L170.
5) Once you've managed to get your unit test passing, verify the fix in the UI as well, just to be sure.

If you have any questions or trouble with any of the above feel free to join us in IRC (irc://irc.mozilla.org/#balrog) to ask.

Comment 18

[Tomislav Jurin]

I have sent you pull request on github, since I have no idea how to make commit appear here. I have put you as reviewer in comment and Bug ID in commit message so you should spot it easily.

I think I have found solution using any_of, tested it in local script and it should work now.

I still need to make test for in ( in balrog , as we discussed ) , but I would like to hear your review of my code anyway. 

Also please leave instructions how to make pull request "correct way ".

Thanks :)

Comment 19

[[github robot]]

Commit pushed to master at https://github.com/mozilla/balrog

https://github.com/mozilla/balrog/commit/20afea59245da2349b6a27f95d4894b23dec8de9
bug 1282841: can't set background rate to 0 for new rules (#189). r=bhearsum

Comment 20

[[e-mail] (:bhearsum)]

This is in production now, thanks Tomislav!