mozilla :: #balrog

16 May 2017
13:30bhearsumg'day folks
13:31aksareengday bhearsum. how's it going ?
13:32bhearsumnot bad, just getting caught up on e-mail
13:32bhearsumhow're you?
13:33aksareeni'm doing fine.studying for test tomorrow
13:38aksareenbhearsum: I updated the PR over the weekend. can you review it ?
13:38bhearsumyep!
13:38dilekHi Ben :)
13:38dilekI'm here.
13:39bhearsumaksareen: are there any outstanding issues, or do you think everything is addressed?
13:39bhearsumoh hey dilek, how're you today?
13:39dilekI'm fine :) are you ?
13:40bhearsumquite well
13:40bhearsumjust give me a few minutes to reread your pull request and e-mail, then we should be able to get things sorted oun
13:40bhearsum*out
13:40dilekI think the view data button is complete :)
13:41dilekOf course :)
13:41dilekI will wait you
13:48aksareenbhearsum: everything we've discussed so far has been addressed
13:49bhearsumaksareen: ok, cool - i'll look shortly!
13:50bhearsumdilek: sorry about the delay - so i still think the "update" and "delete" cases i talked about in https://github.com/mozilla/balrog/pull/304#pullrequestreview-34036446 are not right
13:50bhearsumwhen i click "View Data" for a scheduled deletion, it just shows me "null": https://people-mozilla.org/~bhearsum/sattap/84648b52.png
13:50bhearsumwhich isn't useful information
13:51bhearsumand when i click "View Data" for as cheduled update, it shows me the entire Release instead of a diff between the current data and the new data
13:51dilekone minute ben
13:51bhearsumsure
13:51dilekI work properly
13:52dilekI will take a photo
13:52dilekfile:///home/dilek/Desktop/mozillaaaaa.png
13:53dilekI wonder why
13:54dilekIt work properly
13:54bhearsumyou'll need to upload that somewhere, i can't see your local files
13:54bhearsumor you can e-mail it to me
13:54dilekokey i send e-mail to you
13:55dilekI send
13:56dilek*sent
13:56bhearsumdilek: so it looks like that screenshot is for a scheduled insert
13:56bhearsumthat case works fine for me, too
13:56bhearsumtry scheduling an Update or a Delete on http://localhost:8080/releases
13:57dilekone minute :(
13:59dilekI deleted
14:01dilekHttp: // localhost: 8080 / releases There is an error here
14:01dilek*http://localhost:8080/releases
14:01bhearsumwhat's the error?
14:03dilekI sent e-mail
14:04dilekI fixed my patch, then it was this error ?
14:04bhearsumtwo things:
14:05bhearsum1) you need to *schedule a delete*, not do it directly
14:05bhearsum2) you cannot delete a release that has rules pointing at it, like the error says. you'll need to go to http://localhost:8080/rules and remove the rules pointing at CDM-15 before you can schedule the delete
14:06dilekokey I understand
14:07dilekI have to do now:
14:07dilekHttps://people-mozilla.org/~bhearsum/sattap/84648b52.png I need to fix this
14:07dilekDo i get it right
14:08bhearsumyes, that's one of the cases you need to fix
14:08bhearsumyou also need to fix the "Schedule an Update" case
14:08bhearsumdo you understand what we want to be displayed for the both cases?
14:09dilekwhat you want to be displayed for the both cases? Please tell me and
14:11dilekwe said "update" can be another error record ??
14:11dilekat e-mail ?
14:19bhearsumi described this in https://github.com/mozilla/balrog/pull/304#pullrequestreview-34036446 - is there something not clear about it?
14:20bhearsumdilek: ^
14:22dilekI need to fix "release" part
14:25bhearsumyou need to address the 2nd and 3rd bullet points about the "update" and "delete" scheduled changes
15:11bhearsumaksareen: in https://github.com/mozilla/balrog/pull/312/files/9532932255085431f34fbbaa183437ac8fe0ca8f#r116769075, are you saying you need separate methods solely for the better error messages?
15:13aksareenAnd different intger range checks too. Priority has range (0, infinity) but rate has (0,100)
15:13aksareenbhearsum: ^
15:13bhearsumyeah
15:13bhearsumi think you can parameterize all of that - if you pass min, max, and the field name to a helper function
15:17aksareenbhearsum: I'll have to see. Because AFAIK the format examples in jsonschema were only validating the field values based on format checker function. They cannot access the field name which is in upper level in the tree hierarchy and also the since the field names are decided by user for different models therefore the format checker method will be constrained
15:17aksareento a particular model's field only.
15:20aksareenbhearsum: also minimum and maximum in schema aren't supported by strings, only integers support it. and since type is [string,int,null] the swagger parser throws an error when max or min is specified.
15:20bhearsumi don't think you need to grab it there. i'm thinking something like: https://gist.github.com/mozbhearsum/11affe8a0bfb404f349afc588caa23b7
15:20bhearsumvery similar to what you're doing already, except you can factor out the bodies of those validators
15:25aksareenbhearsum: ill have to see on If and how can i pass the minimum and maximum values to the function ? Also I'd have to check how the wrapper method is invoked and is passes values. Since wrapper is invoked by jsonschema hence the values must be passed from jsonschema's validate format method. But in order to do that how to pass the minimum and maximum values
15:25aksareento jsonschema in the first place ? Without using the minimum and maximum in swagger specification. That's the conundrum. If it were integer it was single line change.
15:26bhearsumi&#39;m not sure i understand? i don&#39;t think the swagger <-> validator interface is changing at all
15:27bhearsumyou&#39;re still only receiving the value from connexion
15:27bhearsumright now you&#39;re hardcoding the min and max in the validator, the only difference in my suggestion is that you&#39;d be passing that to a helper function (that is *not* a validator) to actually do the checking
15:31aksareenbhearsum: oh I think I understand. So I can add the helper method in file and invoke it when API method is invoked from rules.py passing field name value and min max values. Is that right ?
15:33bhearsumhmmm
15:34bhearsumi think i&#39;m still talking about having proper validators, just moving where the bodies of them are
15:34bhearsumyou saw https://gist.github.com/mozbhearsum/11affe8a0bfb404f349afc588caa23b7 ?
15:36aksareenbhearsum: oh I realized I didn&#39;t see the whole file. Sorry about that. Got the idea
15:36bhearsumnp!
17:04dilekbhearsum: I will look at the them. Can I reach you at IRC until time?
17:06bhearsumdilek: i&#39;ll be around for another 4 hours
17:37dilekokey bhearsum, thank you, see you again :)
17:40bhearsumsee you!
17 May 2017
No messages
   
Last message: 8 days and 13 hours ago