webpages_playback.py: Support storing to local directories

webpages_playback.py: Support storing to local directories

Make webpages_playback support storing to local directories as an
alternative to Google Storage.

Rename argument --dest_gsbase to --data_store=<location>. Make this parameter
accept either gs:// url or directory for local file system storage.

Rename argument --upload_to_gs to --upload.

Implement file existence check, download and upload for the file
operation.
NOTREECHECKS=true
NOTRY=true

Committed: https://skia.googlesource.com/skia/+/b4ee7ea834db121bebcff5dc265fee776d18b773

[Kimmo Kinnunen, 2014-12-03 13:49:44]

kkinnunen@nvidia.com changed reviewers:
+ rmistry@google.com

[Kimmo Kinnunen, 2014-12-03 13:49:44]

Would this be ok?
I wasn't able to test the Google Storage -part at all, so if this would be
merged, you'd need to try it out or review with extreme care.. :)

[rmistry, 2014-12-03 14:07:14]

On 2014/12/03 13:49:44, Kimmo Kinnunen wrote:
> Would this be ok?
> I wasn't able to test the Google Storage -part at all, so if this would be
> merged, you'd need to try it out or review with extreme care.. :)

I will need time to review this, should be able to get back to you either
tomorrow or day after.

[Kimmo Kinnunen, 2014-12-18 06:09:37]

On 2014/12/03 14:07:14, rmistry wrote:
> On 2014/12/03 13:49:44, Kimmo Kinnunen wrote:
> > Would this be ok?
> > I wasn't able to test the Google Storage -part at all, so if this would be
> > merged, you'd need to try it out or review with extreme care.. :)
> 
> I will need time to review this, should be able to get back to you either
> tomorrow or day after.

Just a reminder ping..

[rmistry, 2014-12-29 12:28:59]

Sorry for the delay. I patched in this CL and everything appears to work. LGTM.

https://chromiumcodereview.appspot.com/737123005/diff/40001/tools/skp/webpage...
File tools/skp/webpages_playback.py (right):

https://chromiumcodereview.appspot.com/737123005/diff/40001/tools/skp/webpage...
tools/skp/webpages_playback.py:409: class GSWithBucket:
The following is just a suggestion let me know if you think it might be
overkill:

Could you create an abstract class for these two classes to inherit from?
something generic like DataStore and the subclasses can be something like
LocalFileSystem and GoogleStorage (or better names).
The abstract class can probably implement target_name and target_type and throw
exceptions for the ones it expects subclasses to implement.

[Kimmo Kinnunen, 2015-03-27 14:06:54]

On 2014/12/29 12:28:59, rmistry wrote:
> Sorry for the delay. I patched in this CL and everything appears to work.
LGTM.
> 
>
https://chromiumcodereview.appspot.com/737123005/diff/40001/tools/skp/webpage...
> File tools/skp/webpages_playback.py (right):
> 
>
https://chromiumcodereview.appspot.com/737123005/diff/40001/tools/skp/webpage...
> tools/skp/webpages_playback.py:409: class GSWithBucket:
> The following is just a suggestion let me know if you think it might be
> overkill:
> 
> Could you create an abstract class for these two classes to inherit from?
> something generic like DataStore and the subclasses can be something like
> LocalFileSystem and GoogleStorage (or better names).
> The abstract class can probably implement target_name and target_type and
throw
> exceptions for the ones it expects subclasses to implement.

Thanks for looking at this. How about now?

[Kimmo Kinnunen, 2015-03-31 07:18:00]

The CQ bit was checked by kkinnunen@nvidia.com

[Kimmo Kinnunen, 2015-03-31 07:18:00]

The patchset sent to the CQ was uploaded after l-g-t-m from rmistry@google.com
Link to the patchset: https://chromiumcodereview.appspot.com/737123005/#ps80001
(title: "rebase")

[commit-bot: I haz the power, 2015-03-31 07:18:16]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/737123005/80001

[commit-bot: I haz the power, 2015-03-31 07:18:31]

Committed patchset #5 (id:80001) as
https://skia.googlesource.com/skia/+/b4ee7ea834db121bebcff5dc265fee776d18b773

[rmistry, 2015-03-31 13:02:24]

On 2015/03/31 07:18:31, I haz the power (commit-bot) wrote:
> Committed patchset #5 (id:80001) as
> https://skia.googlesource.com/skia/+/b4ee7ea834db121bebcff5dc265fee776d18b773

Update-

There was a bug in this:
http://build.chromium.org/p/client.skia.fyi/builders/Housekeeper-Nightly-Recr...

I fixed it with: https://codereview.chromium.org/1041003003

The next build succeeded:
http://build.chromium.org/p/client.skia.fyi/builders/Housekeeper-Nightly-Recr...


Thanks for this change, it will make running this script easier when you do not
have GS access.

[Kimmo Kinnunen, 2015-03-31 13:11:34]

On 2015/03/31 13:02:24, rmistry wrote:
> There was a bug in this:
>
http://build.chromium.org/p/client.skia.fyi/builders/Housekeeper-Nightly-Recr...
> 
> I fixed it with: https://codereview.chromium.org/1041003003
Thanks, sorry for the inconvenience.