appengine/findit/handlers/flake/check_flaky.py - Issue 2042043004: Added skeleton code for the Detection of regression range for flaky

Side by Side Diff: appengine/findit/handlers/flake/check_flaky.py

Issue 2042043004: Added skeleton code for the Detection of regression range for flaky (Closed) Base URL: https://chromium.googlesource.com/infra/infra.git@master

Patch Set: initial Created 4 years, 6 months ago

Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.

Jump to:

View unified diff | Download patch

OLD	NEW
(Empty)
	1 # Copyright 2016 The Chromium Authors. All rights reserved.

	2 # Use of this source code is governed by a BSD-style license that can be

	3 # found in the LICENSE file.

	4

	5 from common.base_handler import BaseHandler

	6 from common.base_handler import Permission

	7 from waterfall import swarming_util

	8

	9 class CheckFlaky(BaseHandler):
	stgao 2016/06/08 17:24:12 style nit: one more line above. style nit: one more line above. caiw 2016/06/13 22:43:56 Acknowledged. Show quoted text On 2016/06/08 17:24:12, stgao wrote: > style nit: one more line above. Acknowledged.
	10 PERMISSION_LEVEL = Permission.CORP_USER

	11

	12 def HandleGet(self):

	13 #get input parameters
	lijeffrey 2016/06/08 18:05:56 Style nit: comments should be in the form # Get Style nit: comments should be in the form # Get input parameters. <-- space after #, capital first letter, and end with some punctuation. Before submitting your code for review you can run gpylint <path to .py file> And it'll find most style nits for you caiw 2016/06/13 22:43:55 Acknowledged. Show quoted text On 2016/06/08 18:05:56, lijeffrey wrote: > Style nit: > > comments should be in the form > > # Get input parameters. <-- space after #, capital first letter, and end with > some punctuation. > > Before submitting your code for review you can run > > gpylint <path to .py file> > > And it'll find most style nits for you Acknowledged.
	14 master_name = self.request.get('master_name').strip()

	15 builder_name = self.request.get('builder_name').strip()

	16 build_number = self.request.get('build_number').strip()

	17 test_target_name = self.request.get('test_target_name').strip()

	18 testcase_list = self.request.get('testcase_list').strip()
	stgao 2016/06/08 17:24:12 To make it simpler, how about starting with a sing To make it simpler, how about starting with a single testcase? If we do multiple testcases, the Swarming reruns will be not that straightforward. Because different flaky tests might have different regression ranges. caiw 2016/06/13 22:43:56 Acknowledged. Show quoted text On 2016/06/08 17:24:12, stgao wrote: > To make it simpler, how about starting with a single testcase? > > If we do multiple testcases, the Swarming reruns will be not that > straightforward. Because different flaky tests might have different regression > ranges. Acknowledged. stgao 2016/06/13 22:56:11 It seems that this comment was not addressed in th Show quoted text On 2016/06/13 22:43:56, caiw wrote: > On 2016/06/08 17:24:12, stgao wrote: > > To make it simpler, how about starting with a single testcase? > > > > If we do multiple testcases, the Swarming reruns will be not that > > straightforward. Because different flaky tests might have different regression > > ranges. > > Acknowledged. It seems that this comment was not addressed in the latest patch. What's your thought?
	19 print(master_name, builder_name, build_number,
	lijeffrey 2016/06/08 18:05:56 This print looks like it's for debugging. Be sure This print looks like it's for debugging. Be sure to remove it before committing the code. If it's actual logging you want you can use the logging module and look at some of the other code in Findit for how to use it caiw 2016/06/13 22:43:55 Acknowledged. Show quoted text On 2016/06/08 18:05:56, lijeffrey wrote: > This print looks like it's for debugging. Be sure to remove it before committing > the code. > > If it's actual logging you want you can use the logging module and look at some > of the other code in Findit for how to use it Acknowledged. caiw 2016/06/13 22:43:56 Acknowledged. Show quoted text On 2016/06/08 18:05:56, lijeffrey wrote: > This print looks like it's for debugging. Be sure to remove it before committing > the code. > > If it's actual logging you want you can use the logging module and look at some > of the other code in Findit for how to use it Acknowledged.
	20 test_target_name, testcase_list)

	21

	22 # get task id
	chanli 2016/06/08 20:46:00 Style nit like ln 13 Style nit like ln 13 caiw 2016/06/13 22:43:55 Acknowledged. Show quoted text On 2016/06/08 20:46:00, chanli wrote: > Style nit like ln 13 Acknowledged. caiw 2016/06/13 22:43:55 Acknowledged. Show quoted text On 2016/06/08 20:46:00, chanli wrote: > Style nit like ln 13 Acknowledged.
	23 tasks = swarming_util.ListSwarmingTasksDataByTags(
	stgao 2016/06/08 17:24:12 As we only need the task id for the specific test As we only need the task id for the specific test step (implied by the test_target_name), we could filter by master_name, builder_name, build_number, and step_name or name(test_target_name). stgao 2016/06/08 17:24:12 This will send one or more http request to the Swa This will send one or more http request to the Swarming server, and it might take a while. In order to avoid the 1-minute deadline for the frontend http request, it seems better to move this to the App Engine task or App Engine pipeline which will run on the module waterfall-backend. caiw 2016/06/13 22:43:55 Acknowledged. Show quoted text On 2016/06/08 17:24:12, stgao wrote: > This will send one or more http request to the Swarming server, and it might > take a while. In order to avoid the 1-minute deadline for the frontend http > request, it seems better to move this to the App Engine task or App Engine > pipeline which will run on the module waterfall-backend. Acknowledged. caiw 2016/06/13 22:43:56 Acknowledged. Show quoted text On 2016/06/08 17:24:12, stgao wrote: > As we only need the task id for the specific test step (implied by the > test_target_name), we could filter by master_name, builder_name, build_number, > and step_name or name(test_target_name). Acknowledged. caiw 2016/06/13 22:43:56 Acknowledged. Show quoted text On 2016/06/08 17:24:12, stgao wrote: > As we only need the task id for the specific test step (implied by the > test_target_name), we could filter by master_name, builder_name, build_number, > and step_name or name(test_target_name). Acknowledged. caiw 2016/06/14 19:48:05 Acknowledged. Show quoted text On 2016/06/08 17:24:12, stgao wrote: > As we only need the task id for the specific test step (implied by the > test_target_name), we could filter by master_name, builder_name, build_number, > and step_name or name(test_target_name). Acknowledged.
	24 master_name=master_name, builder_name=builder_name,

	25 build_number=build_number,http_client='')

	26 print(tasks)

	27 # TODO: trigger swarming reruns
	lijeffrey 2016/06/08 18:05:57 Nit: for TODOs, if you plan to do them yourself, y Nit: for TODOs, if you plan to do them yourself, you should put your ldap after or a bug number. i.e. # TODO(caiw): Trigger swarming reruns. caiw 2016/06/13 22:43:56 Acknowledged. Show quoted text On 2016/06/08 18:05:57, lijeffrey wrote: > Nit: for TODOs, if you plan to do them yourself, you should put your ldap after > or a bug number. > > i.e. > > # TODO(caiw): Trigger swarming reruns. Acknowledged.
	28 # TODO: get the results from swarming reruns

	29 # TODO: this should return some kind of template

	30

	31 def HandlePost(self): # pragma: no cover

	32 return self.HandleGet()

OLD	NEW

« no previous file with comments | « appengine/findit/handlers/flake/__init__.py ('k') | appengine/findit/main.py » ('j') | appengine/findit/main.py » ('J')