add set_acl() and get_acl() to gs_utils.py

add set_acl() and get_acl() to gs_utils.py

As needed for uploading SKPs and their renderings (see bug 2732)

BUG=skia:2732
R=rmistry@google.com

Committed: https://skia.googlesource.com/common/+/773df58

[epoger, 2014-07-14 22:01:13]

Ravi- please review at patchset 2.

[epoger, 2014-07-15 02:28:05]

Made one more little fix in patchset 3.  Awaiting review.

[rmistry, 2014-07-15 11:26:10]

Exciting stuff

https://codereview.chromium.org/390193002/diff/60001/py/utils/gs_utils.py
File py/utils/gs_utils.py (right):

https://codereview.chromium.org/390193002/diff/60001/py/utils/gs_utils.py#new...
py/utils/gs_utils.py:173: for entry in matching_entries:
Is it possible to ever get a list of more than one entry here? i.e. is this
true: there can be only one entry with the same id_type and id_value.

I see you are asserting for this in get_acl but not in add_acl and delete_acl
and I was wondering why, if that block of code can be identical in all three
methods then we could extract it into a helper method.

https://codereview.chromium.org/390193002/diff/60001/py/utils/gs_utils.py#new...
py/utils/gs_utils.py:231: been set.
[Optional] How about creating a PERMISSION_NONE or PERMISSION_MISSING that is =
None.

[epoger, 2014-07-15 13:20:18]

Thanks for the questions.  Notes for live discussion below...

https://codereview.chromium.org/390193002/diff/60001/py/utils/gs_utils.py
File py/utils/gs_utils.py (right):

https://codereview.chromium.org/390193002/diff/60001/py/utils/gs_utils.py#new...
py/utils/gs_utils.py:173: for entry in matching_entries:
On 2014/07/15 11:26:10, rmistry wrote:
> Is it possible to ever get a list of more than one entry here? i.e. is this
> true: there can be only one entry with the same id_type and id_value.
> 
> I see you are asserting for this in get_acl but not in add_acl and delete_acl
> and I was wondering why, if that block of code can be identical in all three
> methods then we could extract it into a helper method.

AFAICT we should always get either 0 or 1 matching_entries.  I should probably
go ahead and add the assert here; when that assert fails, that will tell me I'm
wrong about that assumption. :-)

https://codereview.chromium.org/390193002/diff/60001/py/utils/gs_utils.py#new...
py/utils/gs_utils.py:231: been set.
On 2014/07/15 11:26:10, rmistry wrote:
> [Optional] How about creating a PERMISSION_NONE or PERMISSION_MISSING that is
=
> None.

I can do that, *if* you think that would also make sense to be passed into
add_acl() as a value of the permission param.  I guess that would replace
delete_acl()...

[epoger, 2014-07-15 13:34:26]

More notes from live discussion... I will create another revision and send it
out for more review later today.

https://codereview.chromium.org/390193002/diff/60001/py/utils/gs_utils.py
File py/utils/gs_utils.py (right):

https://codereview.chromium.org/390193002/diff/60001/py/utils/gs_utils.py#new...
py/utils/gs_utils.py:226: id_value: delete permissions for users whose id_type
field contains this
delete -> get

https://codereview.chromium.org/390193002/diff/60001/py/utils/gs_utils.py#new...
py/utils/gs_utils.py:228: 
Add an example

[epoger, 2014-07-15 19:47:04]

Ravi- PTAL at changes in patchsets 4+5, which I think address the issues we
discussed.

https://codereview.chromium.org/390193002/diff/100001/py/utils/gs_utils.py
File py/utils/gs_utils.py (right):

https://codereview.chromium.org/390193002/diff/100001/py/utils/gs_utils.py#ne...
py/utils/gs_utils.py:209: field = FIELD_BY_ID_TYPE[id_type]
I can extract lines 209-218 into a separate function if you like, but I think
it's actually clearer as-is, because some of those lines would need to be
repeated outside of the extracted function anyway, in set_acl().

[rmistry, 2014-07-16 10:20:54]

LGTM

[epoger, 2014-07-16 12:56:30]

Committed patchset #5 manually as r773df58 (presubmit successful).