[Findit] Adding metadata for manually vs automatically triggered flake analyses

[Findit] Adding metadata for manually vs automatically triggered flake analyses

BUG=654845
Committed: https://chromium.googlesource.com/infra/infra/+/4e1ca15d0fc14bba9b341705a98b9924dbfa8fde

[lijeffrey, 2016-10-11 19:27:00]

Description was changed from

==========
[Findit] Adding metadata for manually vs automatically triggered flake analyses

BUG=
==========

to

==========
[Findit] Adding metadata for manually vs automatically triggered flake analyses

BUG=654845
==========

[lijeffrey, 2016-10-11 19:27:00]

lijeffrey@chromium.org changed reviewers:
+ chanli@chromium.org, stgao@chromium.org

[lijeffrey, 2016-10-11 19:29:40]

ptal

[lijeffrey, 2016-10-13 00:25:02]

ping..

[stgao, 2016-10-13 06:14:38]

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/handlers/f...
File appengine/findit/handlers/flake/check_flake.py (right):

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/handlers/f...
appengine/findit/handlers/flake/check_flake.py:39:
triggering_source=triggering_sources.FINDIT,
Should we do FINDIT_UI?

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/model/flak...
File appengine/findit/model/flake/master_flake_analysis.py (right):

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/model/flak...
appengine/findit/model/flake/master_flake_analysis.py:119:
default=triggering_sources.FINDIT, indexed=True)
The default value is not true for legacy data. Maybe we could just leave it as
None by default.

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/flake/flake_analysis_service.py (right):

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/flake/flake_analysis_service.py:187: itself or Findit
API.
It could be Findit UI, Findit API, and Findit pipeline, right?

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/flake/initialize_flake_pipeline.py (right):

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/flake/initialize_flake_pipeline.py:135:
manually_triggered = user_email.endswith('@google.com')
Not sure if it is a good idea to add this logic deep down in the code path (I
mean from UI/API to here).

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/flake/triggering_sources.py (right):

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/flake/triggering_sources.py:7: # An analysis was
triggered directly through Findit.
Do you mean the UI?

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/flake/triggering_sources.py:21: FINDIT_API: 'Findit
API',
What's the difference here? "Findit API" doesn't belong to "Findit"?

[lijeffrey, 2016-10-14 23:45:22]

comments addressed, ptal :)

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/handlers/f...
File appengine/findit/handlers/flake/check_flake.py (right):

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/handlers/f...
appengine/findit/handlers/flake/check_flake.py:39:
triggering_source=triggering_sources.FINDIT,
On 2016/10/13 06:14:38, stgao wrote:
> Should we do FINDIT_UI?

Done.

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/model/flak...
File appengine/findit/model/flake/master_flake_analysis.py (right):

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/model/flak...
appengine/findit/model/flake/master_flake_analysis.py:119:
default=triggering_sources.FINDIT, indexed=True)
On 2016/10/13 06:14:38, stgao wrote:
> The default value is not true for legacy data. Maybe we could just leave it as
> None by default.

Done.

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/flake/flake_analysis_service.py (right):

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/flake/flake_analysis_service.py:187: itself or Findit
API.
On 2016/10/13 06:14:38, stgao wrote:
> It could be Findit UI, Findit API, and Findit pipeline, right?

Done.

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/flake/initialize_flake_pipeline.py (right):

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/flake/initialize_flake_pipeline.py:135:
manually_triggered = user_email.endswith('@google.com')
On 2016/10/13 06:14:38, stgao wrote:
> Not sure if it is a good idea to add this logic deep down in the code path (I
> mean from UI/API to here).

Done. Changed it back to accept manually_triggered as an input parameter.

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
File appengine/findit/waterfall/flake/triggering_sources.py (right):

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/flake/triggering_sources.py:7: # An analysis was
triggered directly through Findit.
On 2016/10/13 06:14:38, stgao wrote:
> Do you mean the UI?

Done.

https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
appengine/findit/waterfall/flake/triggering_sources.py:21: FINDIT_API: 'Findit
API',
On 2016/10/13 06:14:38, stgao wrote:
> What's the difference here? "Findit API" doesn't belong to "Findit"?

changed to FINDIT_UI, FINDIT_PIPELINE, and FINDIT_API

[chanli, 2016-10-15 01:28:11]

On 2016/10/14 23:45:22, lijeffrey wrote:
> comments addressed, ptal :)
> 
>
https://codereview.chromium.org/2411893002/diff/1/appengine/findit/handlers/f...
> File appengine/findit/handlers/flake/check_flake.py (right):
> 
>
https://codereview.chromium.org/2411893002/diff/1/appengine/findit/handlers/f...
> appengine/findit/handlers/flake/check_flake.py:39:
> triggering_source=triggering_sources.FINDIT,
> On 2016/10/13 06:14:38, stgao wrote:
> > Should we do FINDIT_UI?
> 
> Done.
> 
>
https://codereview.chromium.org/2411893002/diff/1/appengine/findit/model/flak...
> File appengine/findit/model/flake/master_flake_analysis.py (right):
> 
>
https://codereview.chromium.org/2411893002/diff/1/appengine/findit/model/flak...
> appengine/findit/model/flake/master_flake_analysis.py:119:
> default=triggering_sources.FINDIT, indexed=True)
> On 2016/10/13 06:14:38, stgao wrote:
> > The default value is not true for legacy data. Maybe we could just leave it
as
> > None by default.
> 
> Done.
> 
>
https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
> File appengine/findit/waterfall/flake/flake_analysis_service.py (right):
> 
>
https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
> appengine/findit/waterfall/flake/flake_analysis_service.py:187: itself or
Findit
> API.
> On 2016/10/13 06:14:38, stgao wrote:
> > It could be Findit UI, Findit API, and Findit pipeline, right?
> 
> Done.
> 
>
https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
> File appengine/findit/waterfall/flake/initialize_flake_pipeline.py (right):
> 
>
https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
> appengine/findit/waterfall/flake/initialize_flake_pipeline.py:135:
> manually_triggered = mailto:user_email.endswith(
> On 2016/10/13 06:14:38, stgao wrote:
> > Not sure if it is a good idea to add this logic deep down in the code path
(I
> > mean from UI/API to here).
> 
> Done. Changed it back to accept manually_triggered as an input parameter.
> 
>
https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
> File appengine/findit/waterfall/flake/triggering_sources.py (right):
> 
>
https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
> appengine/findit/waterfall/flake/triggering_sources.py:7: # An analysis was
> triggered directly through Findit.
> On 2016/10/13 06:14:38, stgao wrote:
> > Do you mean the UI?
> 
> Done.
> 
>
https://codereview.chromium.org/2411893002/diff/1/appengine/findit/waterfall/...
> appengine/findit/waterfall/flake/triggering_sources.py:21: FINDIT_API: 'Findit
> API',
> On 2016/10/13 06:14:38, stgao wrote:
> > What's the difference here? "Findit API" doesn't belong to "Findit"?
> 
> changed to FINDIT_UI, FINDIT_PIPELINE, and FINDIT_API

lgtm

[lijeffrey, 2016-10-15 01:35:14]

The CQ bit was checked by lijeffrey@chromium.org

[commit-bot: I haz the power, 2016-10-15 01:35:20]

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-10-15 01:49:40]

Description was changed from

==========
[Findit] Adding metadata for manually vs automatically triggered flake analyses

BUG=654845
==========

to

==========
[Findit] Adding metadata for manually vs automatically triggered flake analyses

BUG=654845

Committed:
https://chromium.googlesource.com/infra/infra/+/4e1ca15d0fc14bba9b341705a98b9...
==========

[commit-bot: I haz the power, 2016-10-15 01:49:41]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/infra/infra/+/4e1ca15d0fc14bba9b341705a98b9...