make gs_utils.py work without a .boto credential file

make gs_utils.py work without a .boto credential file

This is needed so developers without a .boto file can still run rebaseline_server.

BUG=skia:2618
R=jcgregorio@google.com, rmistry@google.com

Committed: https://skia.googlesource.com/common/+/ac762a9

[epoger, 2014-07-14 18:41:50]

Ready for review at patchset 2.  First LG wins.

[jcgregorio, 2014-07-14 21:00:08]

On 2014/07/14 18:41:50, epoger wrote:
> Ready for review at patchset 2.  First LG wins.

LGTM, but are you sure failing back to public access is the best. Could this
lead to hard to debug failures further down the road?

[epoger, 2014-07-14 21:14:26]

On 2014/07/14 21:00:08, jcgregorio wrote:
> On 2014/07/14 18:41:50, epoger wrote:
> > Ready for review at patchset 2.  First LG wins.
> 
> LGTM, but are you sure failing back to public access is the best. Could this
> lead to hard to debug failures further down the road?

That's a good question.  Here are 3 approaches I can think of... guys, please
vote for A, B, C, or some other approach I haven't thought of.

A: The way I did it here is the same as the "gsutil" tool does... if the
credentials file is not found, just keep going and hope the user doesn't try to
access any files they need credentials for.  (If the credentials file is not
found, I do log that, so at least we would have that information in the log in
such a case.)

B: A different way to approach it would be: crash if boto file is not found at
the given path.  In that case, tools like rebaseline_server which currently do
not need credentials would be required to call the GSUtils constructor with
boto_file_path=None  ("I am explicitly asserting that I will not need
credentials for what I am about to do.")  Otherwise, it would crash for users
without a .boto file who try to run it.

C: Another thought: maybe the default boto_file_path should be None?  This
library is intended for automated use, after all, which could be an argument in
favor of "the caller to the library must provide the path to the credentials
file, if one is needed".  Then, if the caller has specified a boto_file_path, we
crash if it is not found.

[epoger, 2014-07-15 15:15:58]

On 2014/07/14 21:14:26, epoger wrote:
> On 2014/07/14 21:00:08, jcgregorio wrote:
> > On 2014/07/14 18:41:50, epoger wrote:
> > > Ready for review at patchset 2.  First LG wins.
> > 
> > LGTM, but are you sure failing back to public access is the best. Could this
> > lead to hard to debug failures further down the road?
> 
> That's a good question.  Here are 3 approaches I can think of... guys, please
> vote for A, B, C, or some other approach I haven't thought of.
> 
> A: The way I did it here is the same as the "gsutil" tool does... if the
> credentials file is not found, just keep going and hope the user doesn't try
to
> access any files they need credentials for.  (If the credentials file is not
> found, I do log that, so at least we would have that information in the log in
> such a case.)
> 
> B: A different way to approach it would be: crash if boto file is not found at
> the given path.  In that case, tools like rebaseline_server which currently do
> not need credentials would be required to call the GSUtils constructor with
> boto_file_path=None  ("I am explicitly asserting that I will not need
> credentials for what I am about to do.")  Otherwise, it would crash for users
> without a .boto file who try to run it.
> 
> C: Another thought: maybe the default boto_file_path should be None?  This
> library is intended for automated use, after all, which could be an argument
in
> favor of "the caller to the library must provide the path to the credentials
> file, if one is needed".  Then, if the caller has specified a boto_file_path,
we
> crash if it is not found.

After live discussion, I'm going to go with "C".  New patchset coming soon.

[epoger, 2014-07-15 17:25:05]

See patchset 3 with the changes discussed.  First re-LG wins.

[epoger, 2014-07-16 13:03:14]

Someone (anyone) PTAL.

Patchset 4 merges in my https://codereview.chromium.org/390193002/ ('add
set_acl() and get_acl() to gs_utils.py'), which I committed a few minutes ago.

I'll commit at the next LG, or if I don't hear any complaints for a few hours.

[rmistry, 2014-07-16 13:34:21]

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

https://codereview.chromium.org/390133002/diff/60001/py/utils/gs_utils.py#new...
py/utils/gs_utils.py:79: calling_format=SubdomainCallingFormat(),
provider='google',
Could you explain the provider='google' ?

[epoger, 2014-07-16 13:47:42]

Thanks, Ravi, please see patchset 5

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

https://codereview.chromium.org/390133002/diff/60001/py/utils/gs_utils.py#new...
py/utils/gs_utils.py:79: calling_format=SubdomainCallingFormat(),
provider='google',
On 2014/07/16 13:34:21, rmistry wrote:
> Could you explain the provider='google' ?

Good question.  I have tried to do so in patchset 5...

[rmistry, 2014-07-16 13:55:01]

LGTM

https://codereview.chromium.org/390133002/diff/80001/py/utils/gs_utils.py
File py/utils/gs_utils.py (right):

https://codereview.chromium.org/390133002/diff/80001/py/utils/gs_utils.py#new...
py/utils/gs_utils.py:73: """The GSConnection class constructor in
The Nittiest of Nits:
The style guide says to have a one line summary followed by a blank line and
then more description.

"""GSConnection class that allows anonymous connections.

Your current description.
"""

[epoger, 2014-07-16 14:02:03]

https://codereview.chromium.org/390133002/diff/80001/py/utils/gs_utils.py
File py/utils/gs_utils.py (right):

https://codereview.chromium.org/390133002/diff/80001/py/utils/gs_utils.py#new...
py/utils/gs_utils.py:73: """The GSConnection class constructor in
On 2014/07/16 13:55:01, rmistry wrote:
> The Nittiest of Nits:
> The style guide says to have a one line summary followed by a blank line and
> then more description.
> 
> """GSConnection class that allows anonymous connections.
> 
> Your current description.
> """

I'm not the biggest fan of that directive in the style guide.  ("You have to
indent this much, and you only get 80 chars per line, and you have to fit the
description in one line.")  I am tempted to describe each function only as "See.
Run. Love."

But thank you for suggesting a reasonable one-liner to put at the top.  Done.

[epoger, 2014-07-16 14:02:32]

Committed patchset #6 manually as rac762a9 (presubmit successful).