Summary:
Ref T13258. Implements the "Restartable" behavior, to control whether a build may be restarted or not.
This is fairly straightforward because there are already other existing reasons that a build may not be able to be restarted.
Test Plan: Restarted a build. Marked it as not restartable, saw "Restart" action become disabled. Tried to restart it anyway, got a useful error message.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13258
Differential Revision: https://secure.phabricator.com/D20230
Summary:
Ref T13258. Fixes T11415. This makes "Runnable" actually do something:
- With "Runnable" set to "If Editable" (default): to manually run, pause, resume, abort, or restart a build, you must normally be able to edit the associated build plan.
- If you toggle "Runnable" to "If Viewable", anyone who can view the build plan may take these actions.
This is pretty straightforward since T9614 already got us pretty close to this ruleset a while ago.
Test Plan:
- Created a Build Plan, set "Can Edit" to just me, toggled "Runnable" to "If Viewable"/"If Editable", tried to take actions as another user.
- With "If Editable", unable to run, pause, resume, abort, or restart as another user.
- With "If Viewable", those actions work.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13258, T11415
Differential Revision: https://secure.phabricator.com/D20229
Summary:
Depends on D20218. Ref T13258. It's somewhat cumbersome to get from build plans to related builds but this is a reasonable thing to want to do, so make it a little easier.
Also clean up / standardize / hint a few things a little better.
Test Plan: {F6244116}
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13258
Differential Revision: https://secure.phabricator.com/D20219
Summary:
Depends on D20179. Ref T13088. See PHI351. See PHI1018. In various cases, unit tests names are 19 paths mashed together.
This is probably not an ideal name, and the test harness should probably pick a better name, but if users are fine with it and don't want to do the work to summarize on their own, accept them. We may summarize with "..." in some cases depending on how this fares in the UI.
The actual implementation is a separate "strings" table which is just `<hash-of-string, full-string>`. The unit message table can end up being mostly strings, so this should reduce storage requirements a bit.
For now, I'm not forcing a migration: new writes use the new table, existing rows retain the data. I plan to provide a migration tool, recommend migration, then force migration eventually.
Prior to that, I'm likely to move at least some other columns to use this table (e.g., lint names), since we have a lot of similar data (arbitrarily long user string constants that we are unlikely to need to search or filter).
Test Plan: {F6213819}
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D20180
Summary: Ref T13088. Prepares for putting test names in a separate table to release the 255-character limit.
Test Plan: Viewed revisions, buildables, builds, test lists, specific tests.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D20179
Summary:
Depends on D19919. Ref T11351. This method appeared in D8802 (note that "get...Object" was renamed to "get...Transaction" there, so this method was actually "new" even though a method of the same name had existed before).
The goal at the time was to let Harbormaster post build results to Diffs and have them end up on Revisions, but this eventually got a better implementation (see below) where the Harbormaster-specific code can just specify a "publishable object" where build results should go.
The new `get...Object` semantics ultimately broke some stuff, and the actual implementation in Differential was removed in D10911, so this method hasn't really served a purpose since December 2014. I think that broke the Harbormaster thing by accident and we just lived with it for a bit, then Harbormaster got some more work and D17139 introduced "publishable" objects which was a better approach. This was later refined by D19281.
So: the original problem (sending build results to the right place) has a good solution now, this method hasn't done anything for 4 years, and it was probably a bad idea in the first place since it's pretty weird/surprising/fragile.
Note that `Comment` objects still have an unrelated method with the same name. In that case, the method ties the `Comment` storage object to the related `Transaction` storage object.
Test Plan: Grepped for `getApplicationTransactionObject`, verified that all remaining callsites are related to `Comment` objects.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T11351
Differential Revision: https://secure.phabricator.com/D19920
Summary:
Depends on D19918. Ref T11351. In D19918, I removed all calls to this method. Now, remove all implementations.
All of these implementations just `return $timeline`, only the three sites in D19918 did anything interesting.
Test Plan: Used `grep willRenderTimeline` to find callsites, found none.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T11351
Differential Revision: https://secure.phabricator.com/D19919
Summary: Ref T13222. See PHI986. See PHI896. Harbormaster build targets don't currently have a modern "*.search" API, but there's no reason not to provide one (even if some of the use cases are a little bit questionable).
Test Plan: {F6032423}
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13222
Differential Revision: https://secure.phabricator.com/D19841
Summary: Ref T13189. See PHI710. Ref T13088. Fixes T9951. Allow callers to `harbormaster.sendmessage` to specify that the test details are remarkup so they can use rich formatting and include links, files, etc.
Test Plan: {F5840098}
Reviewers: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13189, T13088, T9951
Differential Revision: https://secure.phabricator.com/D19615
Summary:
See PHI766. Ref T13164. Build log chunk processing does a `preg_split()` on slices, but this isn't terribly efficient.
We can get the same count more cheaply by just using `substr_count()` a few times.
(I also tried `preg_match_all()`, which was between the two in speed.)
Test Plan:
- Used `bin/harbormaster rebuild-log --id X --force` to rebuild logs. Verified that the linemap is identical before/after this change.
- Saw local time for the 18MB log in PHI766 drop from ~1.7s to ~900ms, and `preg_split()` drop out of the profiler (we're now spending the biggest chunk of time on `gzdeflate()`).
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13164
Differential Revision: https://secure.phabricator.com/D19545
Summary:
Ref T13124. See PHI531. When a revision is updated, builds against the older diff tend to stop being relevant. Add an option to abort outstanding older builds automatically.
At least for now, I'm adding this as a build step instead of some kind of special checkbox. An alternate implementation would be some kind of "Edit Options" action on plans with a checkbox like `[X] When this build starts, abort older builds.`
I think adding it as a build step is a bit simpler, and likely to lead to greater consistency and flexibility down the road, make it easier to add options, etc., and since we don't really have any other current use cases for "a bunch of checkboxes". This might change eventually if we add a bunch of checkboxes for some other reason.
The actual step activates //before// the build queues, so it doesn't need to wait in queue before it can actually act. T13088 discusses some plans here if this sticks.
Test Plan:
- Created a "Sleep for 120 seconds" build plan and triggered it with Herald.
- Added an "Abort Older Builds" step.
- Updated a revision several times in a row, saw older builds abort.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13124
Differential Revision: https://secure.phabricator.com/D19376
Summary:
Ref T13114. See PHI511. Ref T13072. This makes Buildables, Builds, Targets and Artifacts destructible with `bin/remove destroy`.
This might not be totally exhaustive. In particular:
- File artifacts won't destroy the file. This is sort of okay because file artifacts are currently just a file reference, but probably shouldn't be how things work in the long term.
- `BuildCommand` doesn't get cleaned up, but `BuildMessage` does on `Build`. See T13072 for more.
Test Plan: Used `bin/remove destroy` to nuke a bunch of builds, buildables, etc. Loaded stuff in the web UI and it all looked like it got nuked properly.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13114, T13072
Differential Revision: https://secure.phabricator.com/D19269
Summary:
Depends on D19162. Ref T13088. When a user links to `$1234`, we need to render a default view of the log with a piece at the head, a piece at the end, and a piece in the middle.
We also need to figure out the offset for line 1234, or multiple offsets for "1234-2345".
Since the logic views/reads mostly anticipated this it isn't too much of a mess, although there are a couple of bugs this exposes with view specifications that use combinations of parameters which were previously impossible.
Test Plan: Viewed a large log with no line marker. Viewed `$1`. Viewed `$end`. Viewed `$35-40`, etc. Expanded context around logs.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19163
Summary: Depends on D19151. Ref T13088. While dramatically less exciting than using `lolcat` and less general than `pv`, this should do the job adequately.
Test Plan: Piped a sizable log into `bin/harbormaster write-log` with `--rate 2048`, saw a progress bar. Loaded the log in the web UI and saw it grow as the page reloaded.
Reviewers: yelirekim
Reviewed By: yelirekim
Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19152
Summary:
Depends on D19150. Ref T13088. Allow clients to retrieve information about build logs, including log data, over the API.
(To fetch log data, take the `filePHID` to `file.search`, then issue a normal GET against the URI. Use a `Content-Range` header to get part of the log.)
Test Plan: Ran `harbormaster.log.search`, got sensible-looking results.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19151
Summary: Depends on D19148. Ref T13088. The new rendering always executes range requests for data it needs, and we can satisfy these requests by loading the smallest number of chunks which span that range.
Test Plan: Piped 50,000 lines of Apache log into Harbormaster, viewed it in the new UI, got sensible rendering times and a reasonable amount of data actually going over the wire.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19149
Summary: Depends on D19139. Ref T13088. This doesn't actually work, but is close enough that a skilled attacker might be able to briefly deceive a small child.
Test Plan:
- Viewed some very small logs under very controlled conditions, saw content.
- Larger logs vaguely do something resembling working correctly.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19141
Summary:
Depends on D19138. Ref T13088. When we want to read the last part of a logfile //and show accurate line numbers//, we need to be able to get from byte offsets to line numbers somehow.
Our fundamental unit must remain byte offsets, because a test can emit an arbitrarily long line, and we should accommodate it cleanly if a test emits 2GB of the letter "A".
To support going from byte offsets to line numbers, compute a map with periodic line markers throughout the offsets of the file. From here, we can figure out the line numbers for arbitrary positions in the file with only a constant amount of work.
Test Plan: Added unit tests; ran unit tests.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19139
Summary: Depends on D19137. Ref T13088. This allows `rebuild-log` to skip work if the chunks are already compressed. It also prepares for a future GC which is looking for "text" or "gzip" chunks to throw away in favor of archival into Files; such a GC can use this column to find collectable logs and then write "file" to it, meaning "chunks are gone, this data is only available in Files".
Test Plan: Ran migration, saw logs populate as "text". Ran `rebuild-log`, saw logs rebuild as "gzip".
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19138
Summary: Depends on D19135. Ref T13088. Denormalize the total log size onto the log itself. This makes reasoning about the log at display time easier, and we don't need to fish around in the database as much to figure out what we're dealing with.
Test Plan: Ran `bin/harbormaster rebuild-log`, saw an existing log populate. Ran `bin/harbormaster write-log`, saw new log write with proper length information.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19136
Summary:
Depends on D19134. Ref T13088. Future changes will support API writers, so push the log lock into the Log object.
Allow open/close ("this process is writing to this log") to be separate from live/final ("this log is still generating more data").
Test Plan: Wrote logs with `bin/harbormater write-log` and updated logs with `bin/harbormaster rebuild-log`.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19135
Summary: Depends on D19133. Ref T13088. Allows build logs to be formally destroyed, cleaning up their chunks and file data.
Test Plan:
- Used `bin/remove destroy` to destroy a log, verified chunks and files were removed.
- Used `bin/harbormaster rebuild-log` to force a log to rebuild, verified files were destroyed and regenerated.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19134
Summary: Depends on D19132. Ref T13088. This implements an extremely skeletal dedicated log page with a more-or-less functional "Download Log" button.
Test Plan: Downloaded a recent log. Tried to download an old (un-finalized) log, couldn't. Used `bin/harbormaster write-log` to get a convenient standalone link to a log.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19133
Summary: Depends on D19131. Ref T13088. During log finalization, stream the log into Files to support "Download Log", archive to Files, and API access.
Test Plan: Ran `write-log` and `rebuild-log`, saw Files objects generate with log content and appropriate permissions.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19132
Summary: Depends on D19130. Ref T13088. Currently, when a build log is closed we compress it in the same process. Separate this out into a dedicated worker since the plan is to do a lot more work during finalization, none of which needs to happen inline during builds (or, particuarly, inline during a Conduit call for API writes in the future).
Test Plan: Ran `bin/harbormaster write-log --trace`, saw compression run inline.
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13088
Differential Revision: https://secure.phabricator.com/D19131
Summary:
Ref T13054. Fixes T10746. Fixes T11154. This is really a one-line fix (include `ABORTED` in `BuildEngine->updateBuildable()`) but try to structure the code a little more clearly too and reduce (at least slightly) the number of random lists of status attributes spread throughout the codebase.
Also add a header tag for buildable status.
Test Plan: Aborted a build, saw buildable fail properly.
Subscribers: yelirekim, PHID-OPKG-gm6ozazyms6q6i22gyam
Maniphest Tasks: T13054, T11154, T10746
Differential Revision: https://secure.phabricator.com/D19055
Summary:
Ref PHI261. This moves Harbormaster build status to work more similarly to other modern status types, like Differential revision status, where we try to specify as much behavior on the server as possible so that the client and server can vary independently.
(I don't have any specific plans to make Harbormaster build status configurable on the server side, but it isn't out of the question.)
Test Plan: Ran `harbormaster.querybuilds` (saw same data as before) and `harbormaster.build.search` (saw same data as before, plus new ANSI color data).
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Differential Revision: https://secure.phabricator.com/D18838
Summary: See PHI176. We issue a query with only `artifactIndex` from `BuildTarget`, but don't have an applicable key.
Test Plan: This isn't on the normal Harbormaster execution path so I'm not 100% sure I have a local repro, but will confirm with customer.
Reviewers: amckinley
Reviewed By: amckinley
Subscribers: PHID-OPKG-gm6ozazyms6q6i22gyam
Differential Revision: https://secure.phabricator.com/D18732
Summary: Fixes T11642. Added a 'name' field to the results from harbormaster.build.search.
Test Plan: Went to `/conduit/method/harbormaster.build.search/` and ran a search that would yield results (because otherwise there will be nothing there). Noted that there was, in fact, a name in the results.
Reviewers: yelirekim, #blessed_reviewers, epriestley
Reviewed By: yelirekim, #blessed_reviewers, epriestley
Subscribers: epriestley, yelirekim
Maniphest Tasks: T11642
Differential Revision: https://secure.phabricator.com/D16569
Summary: We deprecate the existing API method used to access build information from the API, but preserve its response structure after calling through to the new method. I've cordoned off the fields I needed to define in order to meet the output structure by putting those fields in a search attachment.
Test Plan:
Used the API console and looked at the list view controller for builds.
Old output structure:
```lang=json
{
"data": [
{
"id": "16823",
"phid": "PHID-HMBD-xghrwfz6luoye5rgc2hq",
"uri": "https://secure.phabricator.com/harbormaster/build/16823/",
"name": "Run Core Tests",
"buildablePHID": "PHID-HMBB-s6ykzm2jzxz4ymduztq3",
"buildPlanPHID": "PHID-HMCP-pcfxcgyoif67l3buc4zt",
"buildStatus": "passed",
"buildStatusName": "Passed"
}
],
"cursor": {
"limit": 100,
"after": "16823",
"before": null
}
}
```
New output structure:
```lang=json
{
"data": [
{
"id": 1,
"type": "HMBD",
"phid": "PHID-HMBD-qpgcmv67tzaauzayzit5",
"uri": "http://ec2-54-165-244-168.compute-1.amazonaws.com/harbormaster/build/1/",
"name": "arc lint + arc unit",
"buildStatusName": "Passed",
"buildablePHID": "PHID-HMBB-qdefith5uakkepqpjr2g",
"buildPlanPHID": "PHID-HMCP-zswbhazb7ipmaf4plygg",
"buildStatus": "passed",
"initiatorPHID": "PHID-USER-rihx4366f3aczsvc2wtb",
"dateCreated": 1450295643,
"dateModified": 1450295644,
"policy": {
"view": "users",
"edit": "users"
}
}
],
"maps": {},
"query": {
"queryKey": null
},
"cursor": {
"limit": 100,
"after": null,
"before": null,
"order": null
}
}
```
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D16356
Summary:
It's only natural for users to be interested their own builds. We are also building in support for other sources of builds, the only formally supported way to run a build right now is via Herald.
In our third party codebase, we designate an application as the "thing" that started builds which are scheduled and managed automatically by phabricator. I believe this is a common practice elsewhere in the codebase when you're at a loss for a real human identity and you need to apply some transactions.
Test Plan: Ran some builds manually and saw them show up under the list of things I've run. Looking up builds based on those that had been started by a herald rule.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D16353
Summary:
We're picking three useful groups of build statuses to provide as default queries:
- Stuff not yet building
- Stuff building
- Stuff which has finished building
These are reasonable buckets for builds since (unlike most objects in phabricatorland) users are generally waiting impatiently for the machine to do something for them, rather than being responsible for doing something with the machine.
Test Plan: clicked around the search engine and enjoyed my defaults
Reviewers: #blessed_reviewers, epriestley
Reviewed By: #blessed_reviewers, epriestley
Subscribers: epriestley
Differential Revision: https://secure.phabricator.com/D16349
Summary:
This supports a few basic use cases that aren't served by the buildable search engine:
- I'm trying to discover when the last time that this particular build plan failed was.
- I want to know if any builds have deadlocked.
- At a glance, I'm more interested in what build plans are running, not which buildables are being built. This is more often than not the case.
Test Plan: {F1744003}
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin, epriestley
Differential Revision: https://secure.phabricator.com/D16347
Summary:
Ref T11153. If you have a build plan like this:
- Lease machine A.
- Lease machine B.
- Run client-tests on machine A.
- Run server-tests on machine B.
...and we get machine A quickly, then finish the tests, we currently do not release machine A until the whole plan finishes.
In the best case, this wastes resources (something else could be using that machine for a while).
In a worse case, this wastes a lot of resources (if machine B is slow to acquire, or the server tests are much slower than the client tests, machine A will get tied up for a really long time).
In the absolute worst case, this might deadlock things.
Instead, release artifacts as soon as no waiting/running steps take them as inputs. In this case, we'd release machine A as soon as we finished running the client tests.
In the case where machines A and B are resources of the same type, this should prevent deadlocks. In all cases, this should improve build throughput at least somewhat.
Test Plan:
I wrote this build plan which runs a "fast" step (10 seconds) and a "slow" step (120 seconds):
{F1691190}
Before the patch, running this build plan held the lease on the "fast" machine for the full 120 seconds, then released both leases at the same time at the very end.
After this patch, I ran this plan and observed the "fast" lease get released after 10 seconds, while the "slow" lease was held for the full 120.
(Also added some `var_dump()` into things to sanity check the logic; it appeared correct.)
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T11153
Differential Revision: https://secure.phabricator.com/D16145
Summary: Ref T10093. Changes must be pushed to staging before they can be landed from the web.
Test Plan:
Changes must be pushed to staging before they can be landed from the web.
{F1161909}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10093
Differential Revision: https://secure.phabricator.com/D15427
Summary:
Ref T5822.
- After a log is closed, compress it if possible.
- Provide `bin/harbormaster archive-logs` to make it easier to change the storage format of logs.
Test Plan:
- Ran `bin/harbormaster archive-logs` on a bunch of logs, compressing and decompressing them without issues (same hashes, same decompressed size across multiple iterations).
- Ran new builds, verified logs were compressed after they closed.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5822
Differential Revision: https://secure.phabricator.com/D15380
Summary: Ref T5822. This will make it easier to compress and archive chunks without needing to hold them in memory.
Test Plan: Ran a build, looked at some logs.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5822
Differential Revision: https://secure.phabricator.com/D15378
Summary:
Ref T10457. Currently, this table is an ad-hoc table, but can easily be turned into a normal table.
This will make iterating over log chunks to compress and archive them easier.
Test Plan: Viewed logs, ran `bin/storage adjust` with no issues.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15376
Summary:
Ref T10457. Currently, every `append()` call necessarily generates queries, and these queries are slightly inefficient if a large block of data is appended to a partial log (they do about twice as much work as they technically need to).
Use `PhutilRope` to buffer `append()` input so the logic is a little cleaner and we could add a rule like "flush logs no more than once every 500ms" later.
Test Plan:
- Ran a build, saw logs.
- Set chunk size very small, ran build, saw logs, verified small logs in database.
{F1137115}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15375
Summary:
Ref T5822. This prepares for inline compression and garbage collection of build logs.
This reduces the API surface area and removes a log from the "wait" step that would just log a message every 15 seconds. (If this is actually useful, I think we find a better way to communicate it.)
Test Plan:
Ran a build, saw a log:
{F1136691}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T5822
Differential Revision: https://secure.phabricator.com/D15371
Summary:
Ref T10457. When tests fail, it currently takes several clicks to figure out //why// they failed.
In this project, map rebuilds and `liberate` are fairly common failure conditions, but verifying that they were the root issue requires jumping into a build, then scrolling through a log.
Instead, display details if they're available.
Test Plan:
Before:
{F1135453}
After:
{F1135454}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9951, T10457
Differential Revision: https://secure.phabricator.com/D15363
Summary: Ref T10457. These lack color and iconography and are difficult to parse. Make them easier to read.
Test Plan:
Before:
{F1135396}
After:
{F1135399}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10457
Differential Revision: https://secure.phabricator.com/D15362
Summary: Fixes T9614. This is kind of silly, but stop users from fighting turf wars over build resources or showing up on an install and just aborting a bunch of builds for the heck of it.
Test Plan:
- Restarted / paused / aborted / etc builds.
- Tried to do the same for builds I didn't have edit permission on the build plan for, got errors.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9614
Differential Revision: https://secure.phabricator.com/D15353
Summary: Fixes T10192. This key improves some common queries and is not currently present.
Test Plan: See discussion in T10192. Verified current query plan of real queries is garbage and improved by adding this key.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T10192
Differential Revision: https://secure.phabricator.com/D15075
Summary:
Fix T9662.
Record who initiated the build, and allow this information as a parameter.
In this implementation, a 're-run' keeps the original initiator, which we maybe not desired?
Test Plan:
Make a HTTP step with initiator.phid, trigger manually, via HM, via ./bin/harbormaster build.
Look at requests made.
Reviewers: epriestley, #blessed_reviewers
Reviewed By: epriestley, #blessed_reviewers
Subscribers: Korvin
Maniphest Tasks: T9662
Differential Revision: https://secure.phabricator.com/D14380
Summary: Ref T9352. See D13635. Build targets can have variables already, but let builds have them too. This mostly enables future use cases (sub-builds, more sophisticated build triggers).
Test Plan: With a custom Herald rule + action like the one in T9352, updated a revision and saw it generate multiple builds with varying parameters.
Reviewers: chad, hach-que
Reviewed By: hach-que
Maniphest Tasks: T9352
Differential Revision: https://secure.phabricator.com/D14222
Summary:
Ref T9252. This is the same as D14201, but for lease stuff instead of resource stuff.
This one is a little heavier but still feels pretty reasonable to me at the end of the day (worker is <1K lines and has a ton of comment stuff).
Also fixes a few random bugs I hit in the task queue.
Test Plan:
- Restarted some Harbormaster builds, saw them go through cleanly.
- Released pre-activation resources/leases.
- Probably still kinda buggy but I'll iron the details out over time.
Logs are starting to look somewhat plausible:
{F855747}
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9252
Differential Revision: https://secure.phabricator.com/D14202
Summary:
Ref T9123. After recent changes, viewing a live build log from the web UI throws a CSRF exception.
Check `start` ("this object is an active log"), not `live` ("this log is open somewhere").
Test Plan: Will push / etc.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9123
Differential Revision: https://secure.phabricator.com/D14185
Summary:
Ref T9123. To run upstream builds in Harbormaster/Drydock, we need to be able to check out `libphutil`, `arcanist` and `phabricator` next to one another.
This adds an "Also Clone: ..." field to Harbormaster working copy build steps so I can type all three repos into it and get a proper clone with everything we need.
This is somewhat upstream-centric and a bit narrow, but I don't think it's totally unreasonable, and most of the underlying stuff is relatively general.
This adds some more typechecking and improves data/type handling for custom fields, too. In particular, it prevents users from entering an invalid/restricted value in a field (for example, you can't "Also Clone" a repository you don't have permission to see).
Test Plan: Restarted build, got a Drydock resource with multiple repositories in it.
Reviewers: chad
Reviewed By: chad
Maniphest Tasks: T9123
Differential Revision: https://secure.phabricator.com/D14183
