Skip to content

Commit

Permalink
[git-webkit] Reduce radar requests in clone
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=274216
rdar://128135181

Reviewed by Dewei Zhu.

Attempt to batch update requests on cloned radar, falling back to smaller updates if the
larger one fails.

* Tools/Scripts/libraries/webkitbugspy/webkitbugspy/mocks/radar.py:
(RadarModel.commit_changes): Increment network call counter.
(RadarClient.radar_for_id): Ditto
(RadarClient.find_radars): Ditto
(RadarClient.milestones_for_component): Ditto
(RadarClient.find_components): Ditto
(RadarClient.create_radar): Ditto
(RadarClient.clone_radar): Ditto
(RadarClient.keywords_for_name): Ditto
(Radar.__init__): Keep track of function calls that result in network requests.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/clone.py:
(Clone.main): Attempt to batch all radar updates into a single request, falling back to smaller
updates if the batched update fails.
* Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/clone_unittest.py:
(TestClone.test_no_reason): Check the number of radar network requests made during the program.
(TestClone.test_basic): Ditto.
(TestClone.test_no_prompt): Ditto.
(TestClone.test_prompt): Ditto.
(TestClone.test_merge_back_no_parent): Ditto.
(TestClone.test_merge_back): Ditto.
(TestClone.test_infer_merge_back): Ditto.

Canonical link: https://commits.webkit.org/279019@main
  • Loading branch information
JonWBedard committed May 20, 2024
1 parent a4d58c8 commit b1a0769
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 35 deletions.
17 changes: 17 additions & 0 deletions Tools/Scripts/libraries/webkitbugspy/webkitbugspy/mocks/radar.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,8 @@ def keywords(self):
return [self.client.parent.keywords[keyword] for keyword in self._issue.get('keywords', [])]

def commit_changes(self):
self.client.parent.request_count += 1

self.client.parent.issues[self.id]['comments'] = [
Issue.Comment(
user=self.client.parent.users[entry.addedBy.email],
Expand Down Expand Up @@ -293,12 +295,16 @@ def __init__(self, parent, authentication_strategy):
self.authentication_strategy = authentication_strategy

def radar_for_id(self, problem_id, additional_fields=None):
self.parent.request_count += 1

found = self.parent.issues.get(problem_id)
if not found:
return None
return RadarModel(self, found, additional_fields=additional_fields)

def find_radars(self, query, return_find_results_directly=False):
self.parent.request_count += 1

result = []
for issue in self.parent.issues.values():
r = RadarModel(self, issue)
Expand All @@ -317,9 +323,12 @@ def find_radars(self, query, return_find_results_directly=False):
return result

def milestones_for_component(self, component, include_access_groups=False):
self.parent.request_count += 1
return list(self.parent.milestones.values())

def find_components(self, request_data):
self.parent.request_count += 1

filters = []
for key in ('name', 'version'):
if key in request_data:
Expand Down Expand Up @@ -347,6 +356,8 @@ def find_components(self, request_data):
return result

def create_radar(self, request_data):
self.parent.request_count += 1

if not all((
request_data.get('title'), request_data.get('description'), request_data.get('component'),
request_data.get('classification'), request_data.get('reproducible'),
Expand Down Expand Up @@ -391,6 +402,8 @@ def create_radar(self, request_data):
return self.radar_for_id(id)

def clone_radar(self, problem_id, reason_text, component=None):
self.parent.request_count += 1

original = self.radar_for_id(problem_id)
if not original:
raise Radar.exceptions.UnsuccessfulResponseException("'{}' does not match a known radar".format(problem_id))
Expand All @@ -412,6 +425,8 @@ def clone_radar(self, problem_id, reason_text, component=None):
))

def keywords_for_name(self, keyword_name):
self.parent.request_count += 1

return [
keyword for name, keyword in self.parent.keywords.items()
if name.startswith(keyword_name)
Expand Down Expand Up @@ -559,6 +574,8 @@ def __init__(self, users=None, issues=None, projects=None, milestones=None):
from mock import patch
self.patches.append(patch('webkitbugspy.radar.Tracker.radarclient', new=lambda s=None: self))

self.request_count = 0


class NoRadar(ContextStack):
top = None
Expand Down
73 changes: 45 additions & 28 deletions Tools/Scripts/libraries/webkitscmpy/webkitscmpy/program/clone.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,40 +309,57 @@ def pick_attr(name, plural, default=None):
sys.stderr.write('Completed clone, but failed to set parent, continuing...\n')
result += 1

if prefix:
raw_clone.title = '{} {}'.format(prefix, raw_issue.title)
raw_clone.commit_changes()
def modify_radar(
cloned=cloned, raw_clone=raw_clone,
raw_issue=raw_issue,
prefix=prefix, milestone=milestone,
category=category, event=event, tentpole=tentpole,
action=lambda: None,
):
if prefix:
raw_clone.title = '{} {}'.format(prefix, raw_issue.title)
action()

try:
raw_clone.priority = raw_issue.priority
raw_clone.resolution = raw_issue.resolution
raw_clone.commit_changes()
except rdar.radarclient().exceptions.UnsuccessfulResponseException:
sys.stderr.write('Completed clone, but failed to set priority and resolution\n')
return 1
try:
raw_clone.priority = raw_issue.priority
raw_clone.resolution = raw_issue.resolution
action()
except rdar.radarclient().exceptions.UnsuccessfulResponseException:
sys.stderr.write('Completed clone, but failed to set priority and resolution\n')
return 1

try:
raw_clone.milestone = milestone
raw_clone.category = category
raw_clone.event = event
raw_clone.tentpole = tentpole
raw_clone.commit_changes()
except rdar.radarclient().exceptions.UnsuccessfulResponseException:
sys.stderr.write('Completed clone, but failed to set milestone, category, event and tentpole\n')
return 1
try:
raw_clone.milestone = milestone
raw_clone.category = category
raw_clone.event = event
raw_clone.tentpole = tentpole
action()
except rdar.radarclient().exceptions.UnsuccessfulResponseException:
sys.stderr.write('Completed clone, but failed to set milestone, category, event and tentpole\n')
return 1

try:
raw_clone.state = 'Analyze'
if raw_clone.resolution:
raw_clone.substate = 'Prepare'
else:
raw_clone.substate = 'Investigate'
sys.stderr.write('{} does not have a resolution\n'.format(issue.link))
sys.stderr.write('Placing {} in {}\n'.format(cloned.link, raw_clone.substate))
action()
except rdar.radarclient().exceptions.UnsuccessfulResponseException:
sys.stderr.write("Completed clone and set milestone, but failed to move to 'Integrate'\n")
return 1
return 0

try:
raw_clone.state = 'Analyze'
if raw_clone.resolution:
raw_clone.substate = 'Prepare'
else:
raw_clone.substate = 'Investigate'
sys.stderr.write('{} does not have a resolution\n'.format(issue.link))
sys.stderr.write('Placing {} in {}\n'.format(cloned.link, raw_clone.substate))
modify_rc = modify_radar()
raw_clone.commit_changes()
except rdar.radarclient().exceptions.UnsuccessfulResponseException:
sys.stderr.write("Completed clone and set milestone, but failed to move to 'Integrate'\n")
return 1
modify_rc = modify_radar(action=lambda raw_clone=raw_clone: raw_clone.commit_changes())
if modify_rc:
return modify_rc
result += modify_rc

print('Moved clone to {} and into {}{}'.format(
raw_clone.milestone.name,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,13 @@ def test_no_reason(self):
issues=bmocks.ISSUES,
projects=bmocks.PROJECTS,
milestones=bmocks.MILESTONES,
), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):
) as mock_radar, OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):

self.assertEqual(255, program.main(
args=('clone', 'rdar://1'),
path=self.path,
))
self.assertEqual(2, mock_radar.request_count)

self.assertEqual(
captured.stderr.getvalue(),
Expand All @@ -71,12 +72,14 @@ def test_basic(self):
issues=bmocks.ISSUES,
projects=bmocks.PROJECTS,
milestones=bmocks.MILESTONES,
), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):
) as mock_radar, OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):

self.assertEqual(0, program.main(
args=('clone', 'rdar://1', '--reason', 'Cloning for a future branch', '--milestone', 'Future'),
path=self.path,
))
self.assertEqual(22, mock_radar.request_count)

tracker = radar.Tracker()
raw_issue = tracker.client.radar_for_id(4)

Expand All @@ -97,12 +100,13 @@ def test_no_prompt(self):
issues=bmocks.ISSUES,
projects=bmocks.PROJECTS,
milestones=bmocks.MILESTONES,
), MockTerminal.input('1'), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):
) as mock_radar, MockTerminal.input('1'), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):

self.assertEqual(255, program.main(
args=('clone', 'rdar://1', '--reason', 'Cloning for an October branch', '--milestone', 'October', '--no-prompt'),
path=self.path,
))
self.assertEqual(5, mock_radar.request_count)

self.assertEqual(
captured.stderr.getvalue(),
Expand All @@ -116,12 +120,14 @@ def test_prompt(self):
issues=bmocks.ISSUES,
projects=bmocks.PROJECTS,
milestones=bmocks.MILESTONES,
), MockTerminal.input('2'), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):
) as mock_radar, MockTerminal.input('2'), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):

self.assertEqual(0, program.main(
args=('clone', 'rdar://1', '--reason', 'Cloning for an October branch', '--milestone', 'October', '--prompt'),
path=self.path,
))
self.assertEqual(22, mock_radar.request_count)

tracker = radar.Tracker()
raw_issue = tracker.client.radar_for_id(4)

Expand Down Expand Up @@ -182,11 +188,12 @@ def test_merge_back_no_parent(self):
issues=issues,
projects=bmocks.PROJECTS,
milestones=bmocks.MILESTONES,
), MockTerminal.input('1'), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):
) as mock_radar, MockTerminal.input('1'), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):
self.assertEqual(255, program.main(
args=('clone', 'rdar://1', '--reason', 'Cloning for an October branch', '--milestone', 'October', '--merge-back'),
path=self.path,
))
self.assertEqual(11, mock_radar.request_count)

self.assertEqual(
captured.stderr.getvalue(),
Expand Down Expand Up @@ -216,11 +223,13 @@ def test_merge_back(self):
issues=issues,
projects=bmocks.PROJECTS,
milestones=bmocks.MILESTONES,
), MockTerminal.input('1'), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):
) as mock_radar, MockTerminal.input('1'), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):
self.assertEqual(0, program.main(
args=('clone', 'rdar://1', '--reason', 'Cloning for an October branch', '--milestone', 'October', '--merge-back'),
path=self.path,
))
self.assertEqual(42, mock_radar.request_count)

tracker = radar.Tracker()
raw_issue = tracker.client.radar_for_id(5)

Expand Down Expand Up @@ -261,14 +270,16 @@ def test_infer_merge_back(self):
issues=issues,
projects=bmocks.PROJECTS,
milestones=bmocks.MILESTONES,
), MockTerminal.input('y', '1'), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):
) as mock_radar, MockTerminal.input('y', '1'), OutputCapture() as captured, patch('webkitbugspy.Tracker._trackers', [radar.Tracker()]):
tracker = radar.Tracker()
tracker.issue(1).add_comment('Committed 1234.10@some-branch (12345678) to some-branch referencing this bug')

self.assertEqual(0, program.main(
args=('clone', 'rdar://1', '--milestone', 'October'),
path=self.path,
))
self.assertEqual(49, mock_radar.request_count)

raw_issue = tracker.client.radar_for_id(5)

self.assertEqual(raw_issue.milestone.name, 'Internal Tools - October')
Expand Down

0 comments on commit b1a0769

Please sign in to comment.