Skip to content
This repository was archived by the owner on Apr 7, 2022. It is now read-only.

Commit d4a0b1c

Browse files
author
epriestley
committed
Remove names from Drydock resources
Summary: Ref T9252. Long ago you sometimes manually created resources, so they had human-enterable names. However, users never make resources manually any more, so this field isn't really useful any more. In particular, it means we write a lot of untranslatable strings like "Working Copy" to the database in the default locale. Instead, do the call at runtime so resource names are translatable. Also clean up a few minor things I hit while kicking the tires here. It's possible we might eventually want to introduce a human-choosable label so you can rename your favorite resources and this would just be a default name. I don't really have much of a use case for that yet, though, and I'm not sure there will ever be one. Test Plan: - Restarted a Harbormaster build, got a clean build. - Released all leases/resources, restarted build, got a clean build with proper resource names. Reviewers: hach-que, chad Reviewed By: hach-que, chad Maniphest Tasks: T9252 Differential Revision: https://secure.phabricator.com/D14213
1 parent b219bcf commit d4a0b1c

13 files changed

+80
-37
lines changed
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
ALTER TABLE {$NAMESPACE}_drydock.drydock_resource
2+
DROP name;

src/applications/drydock/blueprint/DrydockAlmanacServiceHostBlueprintImplementation.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,9 @@ public function allocateResource(
6969

7070
$binding_phid = $binding->getPHID();
7171

72-
$resource = $this->newResourceTemplate($blueprint, $device_name)
72+
$resource = $this->newResourceTemplate($blueprint)
7373
->setActivateWhenAllocated(true)
74+
->setAttribute('almanacDeviceName', $device_name)
7475
->setAttribute('almanacServicePHID', $binding->getServicePHID())
7576
->setAttribute('almanacBindingPHID', $binding_phid)
7677
->needSlotLock("almanac.host.binding({$binding_phid})");
@@ -95,6 +96,15 @@ public function destroyResource(
9596
return;
9697
}
9798

99+
public function getResourceName(
100+
DrydockBlueprint $blueprint,
101+
DrydockResource $resource) {
102+
$device_name = $resource->getAttribute(
103+
'almanacDeviceName',
104+
pht('<Unknown>'));
105+
return pht('Host (%s)', $device_name);
106+
}
107+
98108
public function canAcquireLeaseOnResource(
99109
DrydockBlueprint $blueprint,
100110
DrydockResource $resource,

src/applications/drydock/blueprint/DrydockBlueprintImplementation.php

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,19 @@ abstract public function destroyResource(
237237
DrydockResource $resource);
238238

239239

240+
/**
241+
* Get a human readable name for a resource.
242+
*
243+
* @param DrydockBlueprint Blueprint which built the resource.
244+
* @param DrydockResource Resource to get the name of.
245+
* @return string Human-readable resource name.
246+
* @task resource
247+
*/
248+
abstract public function getResourceName(
249+
DrydockBlueprint $blueprint,
250+
DrydockResource $resource);
251+
252+
240253
/* -( Resource Interfaces )------------------------------------------------ */
241254

242255

@@ -260,16 +273,13 @@ public static function getNamedImplementation($class) {
260273
return idx(self::getAllBlueprintImplementations(), $class);
261274
}
262275

263-
protected function newResourceTemplate(
264-
DrydockBlueprint $blueprint,
265-
$name) {
276+
protected function newResourceTemplate(DrydockBlueprint $blueprint) {
266277

267278
$resource = id(new DrydockResource())
268279
->setBlueprintPHID($blueprint->getPHID())
269280
->attachBlueprint($blueprint)
270281
->setType($this->getType())
271-
->setStatus(DrydockResourceStatus::STATUS_PENDING)
272-
->setName($name);
282+
->setStatus(DrydockResourceStatus::STATUS_PENDING);
273283

274284
// Pre-allocate the resource PHID.
275285
$resource->setPHID($resource->generatePHID());

src/applications/drydock/blueprint/DrydockWorkingCopyBlueprintImplementation.php

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,7 @@ public function allocateResource(
9797
DrydockBlueprint $blueprint,
9898
DrydockLease $lease) {
9999

100-
$resource = $this->newResourceTemplate(
101-
$blueprint,
102-
pht('Working Copy'));
100+
$resource = $this->newResourceTemplate($blueprint);
103101

104102
$resource_phid = $resource->getPHID();
105103

@@ -185,6 +183,13 @@ public function destroyResource(
185183
}
186184
}
187185

186+
public function getResourceName(
187+
DrydockBlueprint $blueprint,
188+
DrydockResource $resource) {
189+
return pht('Working Copy');
190+
}
191+
192+
188193
public function activateLease(
189194
DrydockBlueprint $blueprint,
190195
DrydockResource $resource,

src/applications/drydock/controller/DrydockLeaseController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ protected function buildApplicationCrumbs() {
4444
$this->getApplicationURI('resource/'));
4545

4646
$crumbs->addTextCrumb(
47-
$resource->getName(),
47+
$resource->getResourceName(),
4848
$this->getApplicationURI("resource/{$id}/"));
4949

5050
$crumbs->addTextCrumb(

src/applications/drydock/controller/DrydockLogController.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ protected function buildApplicationCrumbs() {
9191
$this->getApplicationURI('resource/'));
9292

9393
$crumbs->addTextCrumb(
94-
$resource->getName(),
94+
$resource->getResourceName(),
9595
$this->getApplicationURI("resource/{$id}/"));
9696

9797
$crumbs->addTextCrumb(

src/applications/drydock/controller/DrydockResourceViewController.php

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@ public function handleRequest(AphrontRequest $request) {
1515
return new Aphront404Response();
1616
}
1717

18-
$title = pht('Resource %s %s', $resource->getID(), $resource->getName());
18+
$title = pht(
19+
'Resource %s %s',
20+
$resource->getID(),
21+
$resource->getResourceName());
1922

2023
$header = id(new PHUIHeaderView())
2124
->setUser($viewer)

src/applications/drydock/phid/DrydockResourcePHIDType.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ public function loadHandles(
3333
pht(
3434
'Resource %d: %s',
3535
$id,
36-
$resource->getName()));
36+
$resource->getResourceName()));
3737

3838
$handle->setURI("/drydock/resource/{$id}/");
3939
}

src/applications/drydock/storage/DrydockBlueprint.php

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,16 @@ public function destroyResource(DrydockResource $resource) {
162162
}
163163

164164

165+
/**
166+
* @task resource
167+
*/
168+
public function getResourceName(DrydockResource $resource) {
169+
return $this->getImplementation()->getResourceName(
170+
$this,
171+
$resource);
172+
}
173+
174+
165175
/* -( Acquiring Leases )--------------------------------------------------- */
166176

167177

src/applications/drydock/storage/DrydockResource.php

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@ final class DrydockResource extends DrydockDAO
99
protected $status;
1010
protected $until;
1111
protected $type;
12-
protected $name;
1312
protected $attributes = array();
1413
protected $capabilities = array();
1514
protected $ownerPHID;
@@ -30,7 +29,6 @@ protected function getConfiguration() {
3029
'capabilities' => self::SERIALIZATION_JSON,
3130
),
3231
self::CONFIG_COLUMN_SCHEMA => array(
33-
'name' => 'text255',
3432
'ownerPHID' => 'phid?',
3533
'status' => 'text32',
3634
'type' => 'text64',
@@ -51,6 +49,10 @@ public function generatePHID() {
5149
return PhabricatorPHID::generateNewPHID(DrydockResourcePHIDType::TYPECONST);
5250
}
5351

52+
public function getResourceName() {
53+
return $this->getBlueprint()->getResourceName($this);
54+
}
55+
5456
public function getAttribute($key, $default = null) {
5557
return idx($this->attributes, $key, $default);
5658
}
@@ -118,6 +120,16 @@ public function allocateResource() {
118120
'Only new resources may be allocated.'));
119121
}
120122

123+
// We expect resources to have a pregenerated PHID, as they should have
124+
// been created by a call to DrydockBlueprint->newResourceTemplate().
125+
if (!$this->getPHID()) {
126+
throw new Exception(
127+
pht(
128+
'Trying to allocate a resource with no generated PHID. Use "%s" to '.
129+
'create new resource templates.',
130+
'newResourceTemplate()'));
131+
}
132+
121133
$expect_status = DrydockResourceStatus::STATUS_PENDING;
122134
$actual_status = $this->getStatus();
123135
if ($actual_status != $expect_status) {
@@ -135,12 +147,10 @@ public function allocateResource() {
135147
$new_status = DrydockResourceStatus::STATUS_PENDING;
136148
}
137149

138-
$phid = $this->generatePHID();
139-
140150
$this->openTransaction();
141151
try {
142152
try {
143-
DrydockSlotLock::acquireLocks($phid, $this->slotLocks);
153+
DrydockSlotLock::acquireLocks($this->getPHID(), $this->slotLocks);
144154
$this->slotLocks = array();
145155
} catch (DrydockSlotLockException $ex) {
146156
$this->logEvent(
@@ -152,7 +162,6 @@ public function allocateResource() {
152162
}
153163

154164
$this
155-
->setPHID($phid)
156165
->setStatus($new_status)
157166
->save();
158167
} catch (Exception $ex) {

0 commit comments

Comments
 (0)