[Telemetry] Allow custom upload bucket names

[Telemetry] Allow custom upload bucket names

This CL allows using a custom --upload-bucket for output traces and
results, not limited to the default telemetry buckets (output, internal, etc.)

BUG=chromium:633901
Committed: https://chromium.googlesource.com/external/github.com/catapult-project/catapult/+/fa42633c4b1a400ee1b6c1c2991cd30a6f3d06c8

[perezju, 2016-08-12 14:05:53]

perezju@chromium.org changed reviewers:
+ aiolos@chromium.org, nednguyen@google.com

[perezju, 2016-08-12 14:06:17]

The CQ bit was checked by perezju@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-08-12 14:06:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-12 14:33:15]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-08-12 14:33:16]

Dry run: This issue passed the CQ dry run.

[nednguyen, 2016-08-12 17:14:09]

nednguyen@google.com changed reviewers:
+ zhenw@chromium.org

[nednguyen, 2016-08-12 17:14:10]

+Zhen: since me & Kari are out

[nednguyen, 2016-08-12 17:14:27]

Description was changed from

==========
[Telemetry] Allow custom upload bucket names

This CL allows using a custom --upload-bucket for output traces and
results, not limited to the default telemetry buckets (output, internal, etc.)

BUG=chromium:633901
==========

to

==========
[Telemetry] Allow custom upload bucket names

This CL allows using a custom --upload-bucket for output traces and
results, not limited to the default telemetry buckets (output, internal, etc.)

BUG=chromium:633901
==========

[nednguyen, 2016-08-12 17:14:27]

nednguyen@google.com changed reviewers:
- aiolos@chromium.org, nednguyen@google.com

[Zhen Wang, 2016-08-12 17:35:27]

https://codereview.chromium.org/2241603003/diff/1/telemetry/telemetry/interna...
File telemetry/telemetry/internal/story_runner.py (right):

https://codereview.chromium.org/2241603003/diff/1/telemetry/telemetry/interna...
telemetry/telemetry/internal/story_runner.py:325: bucket =
finder_options.upload_bucket
Since now we allow arbitrary bucket name, if the bucket name is not valid (e.g.,
typo), is there a way to do some error handling for that? For example, print out
error message to notify the user?

[perezju, 2016-08-12 17:41:03]

https://codereview.chromium.org/2241603003/diff/1/telemetry/telemetry/interna...
File telemetry/telemetry/internal/story_runner.py (right):

https://codereview.chromium.org/2241603003/diff/1/telemetry/telemetry/interna...
telemetry/telemetry/internal/story_runner.py:325: bucket =
finder_options.upload_bucket
On 2016/08/12 17:35:27, Zhen Wang wrote:
> Since now we allow arbitrary bucket name, if the bucket name is not valid
(e.g.,
> typo), is there a way to do some error handling for that? For example, print
out
> error message to notify the user?

I thought about that. The user will get some log error messages, but right in
the end, after the benchmark has finished running; letting them know that traces
could not be uploaded.

I guess we could try to validate that the bucket is reachable and with the right
permissions right after parsing the command line options?

Will work on that, but probably wont have it till next Wednesday (as I'm flying
+ in training till then).

[Zhen Wang, 2016-08-12 17:48:21]

https://codereview.chromium.org/2241603003/diff/1/telemetry/telemetry/interna...
File telemetry/telemetry/internal/story_runner.py (right):

https://codereview.chromium.org/2241603003/diff/1/telemetry/telemetry/interna...
telemetry/telemetry/internal/story_runner.py:325: bucket =
finder_options.upload_bucket
On 2016/08/12 17:41:02, perezju wrote:
> On 2016/08/12 17:35:27, Zhen Wang wrote:
> > Since now we allow arbitrary bucket name, if the bucket name is not valid
> (e.g.,
> > typo), is there a way to do some error handling for that? For example, print
> out
> > error message to notify the user?
> 
> I thought about that. The user will get some log error messages, but right in
> the end, after the benchmark has finished running; letting them know that
traces
> could not be uploaded.
> 
> I guess we could try to validate that the bucket is reachable and with the
right
> permissions right after parsing the command line options?
> 
> Will work on that, but probably wont have it till next Wednesday (as I'm
flying
> + in training till then).

Where do the custom buckets come from? Is that for local testing or something
else? I am just thinking if it is valuable to know the bucket name is wrong
before running the benchmark. If not, knowing that at the end is also fine.

[perezju, 2016-08-12 18:02:27]

On 2016/08/12 17:48:21, Zhen Wang wrote:
>
https://codereview.chromium.org/2241603003/diff/1/telemetry/telemetry/interna...
> File telemetry/telemetry/internal/story_runner.py (right):
> 
>
https://codereview.chromium.org/2241603003/diff/1/telemetry/telemetry/interna...
> telemetry/telemetry/internal/story_runner.py:325: bucket =
> finder_options.upload_bucket
> On 2016/08/12 17:41:02, perezju wrote:
> > On 2016/08/12 17:35:27, Zhen Wang wrote:
> > > Since now we allow arbitrary bucket name, if the bucket name is not valid
> > (e.g.,
> > > typo), is there a way to do some error handling for that? For example,
print
> > out
> > > error message to notify the user?
> > 
> > I thought about that. The user will get some log error messages, but right
in
> > the end, after the benchmark has finished running; letting them know that
> traces
> > could not be uploaded.
> > 
> > I guess we could try to validate that the bucket is reachable and with the
> right
> > permissions right after parsing the command line options?
> > 
> > Will work on that, but probably wont have it till next Wednesday (as I'm
> flying
> > + in training till then).
> 
> Where do the custom buckets come from? Is that for local testing or something
> else? I am just thinking if it is valuable to know the bucket name is wrong
> before running the benchmark. If not, knowing that at the end is also fine.

See the bug for context. We're enabling an (internal) benchmark that is going
to spew loads and loads of traces. I thought this was best, not to leak the
bucket name (not super sensitive, but anyway), and not pollute the list of
default buckets with ones needed for one-off benchmarks.

I'm not so sure on how important is to validate the bucket name in the
beginning.
It sounds nice to have, to prevent users from wasting time running a benchmark
that won't upload test results as they were expecting. On the other hand, I
think
we'll rarely have users using this option, and most bots will just use the
default
buckets. So?

[Zhen Wang, 2016-08-12 18:16:49]

> See the bug for context. We're enabling an (internal) benchmark that is going
> to spew loads and loads of traces. I thought this was best, not to leak the
> bucket name (not super sensitive, but anyway), and not pollute the list of
> default buckets with ones needed for one-off benchmarks.
> 
> I'm not so sure on how important is to validate the bucket name in the
> beginning.
> It sounds nice to have, to prevent users from wasting time running a benchmark
> that won't upload test results as they were expecting. On the other hand, I
> think
> we'll rarely have users using this option, and most bots will just use the
> default
> buckets. So?

Makes sense. lgtm

[perezju, 2016-08-12 18:21:31]

The CQ bit was checked by perezju@chromium.org

[commit-bot: I haz the power, 2016-08-12 18:21:36]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-08-12 18:37:42]

Description was changed from

==========
[Telemetry] Allow custom upload bucket names

This CL allows using a custom --upload-bucket for output traces and
results, not limited to the default telemetry buckets (output, internal, etc.)

BUG=chromium:633901
==========

to

==========
[Telemetry] Allow custom upload bucket names

This CL allows using a custom --upload-bucket for output traces and
results, not limited to the default telemetry buckets (output, internal, etc.)

BUG=chromium:633901

Committed:
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...
==========

[commit-bot: I haz the power, 2016-08-12 18:37:43]

Committed patchset #1 (id:1) as
https://chromium.googlesource.com/external/github.com/catapult-project/catapu...