1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-12-27 16:00:59 +01:00
Commit graph

97 commits

Author SHA1 Message Date
James Rhodes
ed76c2be1d Implement showing buildable status in Diffusion
Summary: This implements showing the buildable status in Diffusion and unifies some of the logic used to calculate and render build and buildable statuses.

Test Plan: Looked at diffs and commits with statuses, they rendered fine.  Looked at Diffusion and saw buildable status appear (with a manual buildable and manual buildables included in the query).

Reviewers: #blessed_reviewers, epriestley, chad

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9496
2014-06-14 02:28:00 +10:00
epriestley
b8bc0aa2b0 Allow users to select QueryPanel search engines from a list
Summary: Ref T4986. Instead of requiring users to know the name of an application search engine class, let them select from a list.

Test Plan:
Created a new panel.

{F165468}

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T4986

Differential Revision: https://secure.phabricator.com/D9500
2014-06-12 13:22:20 -07:00
James Rhodes
14198d62fb Return the build from applyPlan instead of the plan
Summary: Nothing inside Phabricator uses the return value of this method, but returning the actual build instance is far more useful (for kicking off builds in an application and storing the build PHID against another object).

Test Plan: N/A

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley, #blessed_reviewers

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9494
2014-06-11 20:02:11 -07:00
Joshua Spence
0a62f13464 Change double quotes to single quotes.
Summary: Ran `arc lint --apply-patches --everything` over rP, mainly to change double quotes to single quotes where appropriate. These changes also validate that the `ArcanistXHPASTLinter::LINT_DOUBLE_QUOTE` rule is working as expected.

Test Plan: Eyeballed it.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9431
2014-06-09 11:36:50 -07:00
Chad Little
41ef6824be Make ObjectItem default as "Card"
Summary: This went smoother than expeced. Makes the rounded Card the default, also tweaked selected state a little.

Test Plan:
Test UIExamples, Maniphest, Home, Differential, Harbormaster, Audit. Everything seems normal

{F163971}

{F163973}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9408
2014-06-07 12:12:11 -07:00
epriestley
6df1a02413 (Redesign) Clean up older "Tile" code
Summary:
This does some backend cleanup of the tile stuff, and some general cleanup of other application things:

  - Users who haven't customized preferences get a small, specific set of pinned applications: Differential, Maniphest, Diffusion, Audit, Phriction, Projects (and, for administrators, Auth, Config and People).
  - Old tile size methods are replaced with `isPinnnedByDefault()`.
  - Shortened some short descriptions.
  - `shouldAppearInLaunchView()` replaced by less ambiguous `isLaunchable()`.
  - Added a marker for third-party / extension applications.

Test Plan: Faked away my preferences and viewed the home page, saw a smaller set of default pins.

Reviewers: chad

Reviewed By: chad

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D9358
2014-06-03 15:47:27 -07:00
epriestley
81d95cf682 Make default view of "Applications" app a full-page launcher
Summary:
This probably needs some tweaks, but the idea is to make it easier to browse and access applications without necessarily needing them to be on the homepage.

Open to feedback.

Test Plan:
(This screenshot merges "Organization", "Communication" and "Core" into a single "Core" group. We can't actually do this yet because it wrecks the homepage.)

{F160052}

Reviewers: btrahan, chad

Reviewed By: chad

Subscribers: epriestley

Maniphest Tasks: T5176

Differential Revision: https://secure.phabricator.com/D9297
2014-05-29 12:17:54 -07:00
Chad Little
3a81f8c68d Convert rest of SPRITE_STATUS to FontAwesome
Summary:
Updates policy, headers, typeaheads to FA over policy icons

Need advice - can't seem to place where icons come from on Typeahead? Wrong icons and wrong colors.... it is late

Test Plan:
- grepped for SPRITE_STATUS
- grepped for sprite-status
- grepped for setStatus for headers
- grepped individual icons names

Browsed numerous places, checked new dropdowns, see pudgy people.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4739

Differential Revision: https://secure.phabricator.com/D9179
2014-05-18 16:10:54 -07:00
Chad Little
31cd9b2169 Update PHUIStatusItemView to FontAwesome
Summary: Changes to using FontAwesome

Test Plan:
Testing UIExamples and each of the pages (except releelph)

{F155942}

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D9157
2014-05-16 18:59:02 -07:00
epriestley
702f073f0a Fix some logic in WaitForPreviousBuildStep
Summary: Fixes T5062. See inlines.

Test Plan: Did not test whatsoever.

Reviewers: hach-que

Reviewed By: hach-que

Subscribers: epriestley

Maniphest Tasks: T5062

Differential Revision: https://secure.phabricator.com/D9132
2014-05-15 07:36:44 -07:00
Aviv Eyal
f2c0e94ea8 Show command transactions in Harbormaster builds
Summary:
Create transaction, editor, etc, and move command generation over to editor.
Show in a timeline in the buildable page.

Also prevent Engine from creating an empty transaction when build starts (Fixes T4885).

Fixes T4886.

Test Plan: Restart builds and buildables, look at timeline.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: epriestley, Korvin

Maniphest Tasks: T4885, T4886

Differential Revision: https://secure.phabricator.com/D9110
2014-05-15 07:04:34 -07:00
epriestley
15561a27c3 When a conduit method requires a string constant, call it "string-const" not "enum"
Summary: Ref T5058. The use of "enum" is confusing; we mean "choose one of these specific string constants". Make this more clear.

Test Plan: Viewed each call from the web UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T5058

Differential Revision: https://secure.phabricator.com/D9127
2014-05-14 21:59:03 -07:00
Chad Little
0120388a75 Found some missing icons
Summary: Did a more exhaustive grep on setIcon and found 99.9% of the icons.

Test Plan: I verified icon names on UIExamples, but unable to test some of the more complex flows visually. Mostly a read and replace.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9088
2014-05-13 07:45:39 -07:00
Chad Little
b2f3001ec4 Replace Sprite-Icons with FontAwesome
Summary: The removes the sprite sheet 'icons' and replaces it with FontAwesome fonts.

Test Plan:
- Grep for SPRITE_ICONS and replace
- Grep for sprite-icons and replace
- Grep for PhabricatorActionList and choose all new icons
- Grep for Crumbs and fix icons
- Test/Replace PHUIList Icon support
- Test/Replace ObjectList Icon support (foot, epoch, etc)
- Browse as many pages as I could get to
- Remove sprite-icons and move remarkup to own sheet
- Review this diff in Differential

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin, hach-que

Differential Revision: https://secure.phabricator.com/D9052
2014-05-12 10:08:32 -07:00
epriestley
78b89711cb Move a bunch more rendering into SearchEngine
Summary: Ref T4986. These are mostly mechanical now, I skipped a couple of slightly tricky ones. Still a bunch to go.

Test Plan:
For each engine:

  - Viewed the application;
  - created a panel to issue the query.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4986

Differential Revision: https://secure.phabricator.com/D9017
2014-05-08 20:04:19 -07:00
Chad Little
173fd49e67 Used Cards instead of States for Harbormaster Buildables
Summary: Switched to Obect Cards for better consistency with application search. Added Byline for colorblind/accessability (can move).

Test Plan: Tested my Harbormaster build.

Reviewers: epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8934
2014-05-01 14:38:21 -07:00
epriestley
d41416faf0 Let dashboard panel types use customfield to manage editing
Summary: Ref T3583. Use the same approach Harbormaster does to give panels cheap forms.

Test Plan:
{F149218}

{F149219}

{F149220}

Reviewers: chad, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T3583

Differential Revision: https://secure.phabricator.com/D8919
2014-04-30 14:29:41 -07:00
Chad Little
db42aae361 Add PHUIObjectItemView Status Display to Harbormaster
Summary: Took a short pass here with the new UI, holler if something is TOO EXTREME.

Test Plan:
Tested with manual sleep builds.

{F148693}

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8901
2014-04-29 11:10:16 -07:00
Chad Little
11fd6afeb1 Move Timeline icons to Fonts
Summary: Throwing this up for testing, swapped out all icons in timeline for their font equivelants. Used better icons where I could as well. We should feel free to use more / be fun with the icons when possible since there is no penalty anymore.

Test Plan: I browsed many, not all, timelines in my sandbox and in IE8. Some of these were just swagged, but I'm expecting we'll do more SB testing before landing.

Reviewers: btrahan, epriestley

Reviewed By: epriestley

Subscribers: epriestley, Korvin

Differential Revision: https://secure.phabricator.com/D8827
2014-04-22 08:25:54 -07:00
epriestley
95a405da10 Record build success or failure on buildable objects
Summary:
Fixes T4810. When a buildable completes, make an effort to update the corresponding object with a success or failure message. Commits don't support this yet, but revisions do.

{F144614}

Test Plan:
  - Used `bin/harbormaster build` and `bin/harbormaster update` to run a pile of builds.
  - Tried good/bad builds.
  - Sent some normal mail to make sure the mail reentrancy change didn't break stuff.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4810

Differential Revision: https://secure.phabricator.com/D8803
2014-04-17 16:04:14 -07:00
epriestley
0ef599e906 Give Buildables a status, populate it, and return it over Conduit
Summary:
Ref T4809. Currently, buildables have a status field but nothing populates it. Populate it:

  - When builds change state, update the Buildable state.
  - Use the new Buildable state on the web UI.
  - Return the new Buildable state from Conduit.

To make it easier to debug/test this:

  - Provide `bin/harbormaster update Bxxx ...` to force foreground update of a Buildable.

Test Plan:
  - Used `bin/harbormaster update Bxxx --force --trace` to update buildables.
  - Looked at buidlable list, saw statuses reported properly.
  - Used Conduit to read statuses.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4809

Differential Revision: https://secure.phabricator.com/D8799
2014-04-17 16:01:16 -07:00
epriestley
4918773afe Drop nonsense buildStatus field from Buildable
Summary:
Ref T4809. Buildables currently have buildStatus and buildableStatus. Neither are used, and no one knows why we have two.

I'm going to use buildableStatus shortly, but buildStatus is meaningless; burn it.

Test Plan: `grep`, examined similar get/set calls, created a new buildable, ran storage upgrade.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4809

Differential Revision: https://secure.phabricator.com/D8796
2014-04-17 16:01:06 -07:00
epriestley
7c1bcdea16 Add "harbormaster.querybuilds" Conduit API
Summary:
Ref T4809. This one is more straightforward. A couple of tweaks:

  - Remove the WAITING status, since nothing ever sets it and I suspect nothing ever will with the modern way artifacts work (maybe). At a minimum, it's confusing with the new Target status that's also called "WAITING" but means something different.
  - Consolidate 17 copies of these status names into one method.

Test Plan: Ran some queries via Conduit, got reasonable looking results.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4809

Differential Revision: https://secure.phabricator.com/D8795
2014-04-17 16:00:58 -07:00
epriestley
3b0be0961c Add a rough harbormaster.querybuildables Conduit API method
Summary: Ref T4809. I need to sort out some of the "status" stuff we're doing before this is actually useful (there's no sensible "status" value to expose right now) but once that happens `arc` can query this to figure out whether it needs to warn the user about pending/failed builds.

Test Plan: Ran query with various different parameters.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4809

Differential Revision: https://secure.phabricator.com/D8794
2014-04-17 16:00:25 -07:00
epriestley
4a6d2e9c97 Allow tasks to yield to other tasks
Summary:
For Harbormaster tasks which want to poll or wait, this lets them say "try again a little later" without having to sleep and hold a queue slot.

This is basically the same as failing, except that we don't increment the failure counter. Instead, we just set the current lease to the correct length and then exit. The task will be retried after the lease expires.

Test Plan: Using both `bin/harbormaster` and `phd debug taskmaster`, ran a lot of waiting tasks through the queue, faking them to either yield or not yield in a controlled manner. The queue responded as expected, yielding tasks appropraitely and retrying them later.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8792
2014-04-16 13:02:12 -07:00
epriestley
afd04731ab Add a "Create build step" transaction to Harbormaster
Summary:
Without this, build steps that have no options (like "wait for previous commits") don't actually save, since the transaction array is empty.

This also generally nice and consistent.

Test Plan: Created a new "wait" step, viewed transaction log.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8791
2014-04-16 13:01:56 -07:00
epriestley
78bf266bde Allow Harbormaster build targets to wait for messages
Summary:
This hooks up all the pieces of the build pipeline so `harbormaster.sendmessage` actually works. Particularly:

  - Candidate build steps (i.e., those which interact with external systems) can now "Wait for Message". This pauses them indefinitely when they complete, until something calls `harbormaster.sendmessage`.
  - After processing a target, we check if we should move it to PASSED or WAITING.
  - Before updating a build, we move WAITING targets with pending messages to either PASSED or FAILED.
  - I added an explicit "Building" state, which doesn't affect workflows but communicates more information to human users.

A big part of this is avoiding races. I believe we get the correct behavior no matter which order events occur in:

  - We update builds after targets complete and after we receive messages, so we're guaranteed to update once both these conditions are true. This means messages can't be lost (even if they arrive before a build completes).
  - The minor changes to the build engine logic mean that firing additional build updates is always safe, no matter what the current state of the build is.
  - The build itself is protected by a lock in the build engine.
  - The target is not covered by an explicit lock, but for all states only the engine (waiting) //or// the worker (all other states) can interact with it. All of the interactions also move the target state forward to the same destination and have no other side effects.
  - Messages are only consumed inside the engine lock, so they don't need an explicit lock.

Test Plan:
  - Made an HTTP request wait after completion, then ran a pile of builds through it using `bin/harbormaster build` and the web UI.
  - Passed and failed message-awaiting builds with `harbormaster.sendmessage`.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, zeeg

Differential Revision: https://secure.phabricator.com/D8788
2014-04-16 13:01:46 -07:00
epriestley
803c50c1e7 Allow Harbormaster HTTP steps to pass credentials
Summary: Fixes T4590. Use the credentials custom field to allow Harbormaster HTTP requests to include usernames/passwords.

Test Plan: Ran a build plan with credentials, verified they were sent to the remote server.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4590

Differential Revision: https://secure.phabricator.com/D8786
2014-04-16 13:01:38 -07:00
epriestley
cb545856a9 Make task queue more robust against long-running tasks
Summary:
See discussion in D8773. Three small adjustments which should help prevent this kind of issue:

  - When queueing followup tasks, hold them on the worker until we finish the task, then queue them only if the work was successful.
  - Increase the default lease time from 60 seconds to 2 hours. Although most tasks finish in far fewer than 60 seconds, the daemons are generally stable nowadays and these short leases don't serve much of a purpose. I think they also date from an era where lease expiry and failure were less clearly distinguished.
  - Increase the default wait-after-failure from 60 seconds to 5 minutes. This largely dates from the MetaMTA era, where Facebook ran services with high failure rates and it was appropriate to repeatedly hammer them until things went through. In modern infrastructure, such failures are rare.

Test Plan:
  - Verified that tasks queued properly after the main task was updated.
  - Verified that leases default to 7200 seconds.
  - Intentionally failed a task and verified default 300 second wait before retry.
  - Removed all default leases shorter than 7200 seconds (there was only one).
  - Checked all the wait before retry implementations for anything much shorter than 5 minutes (they all seem reasonable).

Reviewers: btrahan, sowedance

Reviewed By: sowedance

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8774
2014-04-15 08:42:02 -07:00
James Rhodes
fc3b5ddce6 Prevent buildable list in Harbormaster from breaking when container or buildables are missing
Summary: Ref T1049.  I'm fair sure this is just a case of bad data in my local install, but we probably don't want the default page for Harbormaster to break when there's invalid / missing container or buildable handles on any of the builds.

Test Plan: Loaded the page, didn't get a crash due to null reference.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: demo, epriestley, Korvin

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8608
2014-03-25 17:35:49 -07:00
epriestley
6e3c17e6f9 Don't create invalid build steps while adding them
Summary:
Ref T1049. Currently, the "add" dialog lets you select a build step type, but then immediately creates one. If you "cancel" from the edit screen, you end up with an empty (and almost certainly invalid) build step.

Instead, don't create the step until it's valid.

Test Plan: Add Step -> Pick Type -> Add Step -> Cancel no longer creates empty step.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8605
2014-03-25 16:12:05 -07:00
epriestley
d6b937ca27 Allow external systems to send messages to build targets
Summary:
Ref T1049. Allows external systems to send a message to a build target. The primary intended use case is:

  - You make an HTTP request to Jenkins.
  - The build goes into a "waiting" state.
  - Later, Jenkins calls `harbormaster.sendmessage` to report that the target passed or failed.
  - The build continues as appropriate.

This is deceptively complicated because:

  - There are a lot of race concerns. We might get a message back from an external system before it even responds to the request we made. We want to make sure we process these messages no matter when we receive them.
  - These messages need to be sent to a build target (vs a build or buildable) because we'll get into trouble with parallelization later on otherwise (Jenkins is told to do 3 builds; we can't tell which ones failed or what overall state is unless the message are sent to targets).
  - I initially thought about implementing this as a separate "Wait for a response from an external system" build step. This gets a lot more complicated for users once we do parallelization, though. Particularly, in the case where you've told Jenkins to do 3 builds, the three "wait" steps need to know which target they're waiting for (and jenkins needs to know some unique identifier for each target). So this pretty much boils down to a more complicated, more error-prone version of using target PHIDs.

This makes the already-muddy Build UI a bit worse, but it needs a general clarity pass anyway (it's showing way too much uninteresting data, and should show a better summary of results instead).

Test Plan:
  - This doesn't really do anything interesting yet.
  - Used Conduit to send messages to build plans.
  - Viewed the messages on the build screen.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8604
2014-03-25 16:11:28 -07:00
epriestley
25f91567a7 Make various minor Harbormaster UI improvements
Summary: Ref T1049. Tweaks some of the UI and code to improve / clean it up a bit.

Test Plan: Ran build plans, browsed UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8603
2014-03-25 16:10:50 -07:00
epriestley
cec8d10731 Rename concrete Harbormaster step implementations
Summary: Ref T1049. For consistency, rename these to "Harbormaster...".

Test Plan: Ran migration, ran builds, everything still works fine.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8602
2014-03-25 16:09:51 -07:00
epriestley
281f06e281 Rename "BuildStepImplementation" to "HarbormasterBuildStepImplementation"
Summary: Ref T1049. D8588 already required custom code to change what it extends, so this is as good a time as we're going to get to move to more standard class name.

Test Plan: `arc liberate`; `arc lint`

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8601
2014-03-25 16:09:21 -07:00
epriestley
a246c85c6b Use ApplicationTransactions and CustomField to implement build steps
Summary:
Ref T1049. Fixes T4602. Moves all the funky field stuff to CustomField. Uses ApplicationTransactions to apply and record edits.

This makes "artifact" fields a little less nice (but still perfectly usable). With D8599, I think they're reasonable overall. We can improve this in the future.

All other field types are better (e.g., fixes weird bugs with "bool", fixes lots of weird behavior around required fields), and this gives us access to many new field types.

Test Plan:
Made a bunch of step edits. Here's an example:

{F133694}

Note that:

  - "Required" fields work correctly.
  - the transaction record is shown at the bottom of the page.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T4602, T1049

Differential Revision: https://secure.phabricator.com/D8600
2014-03-25 16:08:40 -07:00
epriestley
72337dedaf Make Harbormaster input and output artifacts more explicit
Summary:
Ref T1049. In Harbormaster, build steps may have various inputs (like a host they should run on) and outputs (like a reference to an uploaded file).

  - Currently, inputs aren't defined anywhere (except implicitly at runtime).
    - Instead, define inputs explicitly.
  - Currently, outputs are defined in a way that loses information when misconfigured (the keys will collide).
    - Instead, define inputs and outputs so they work whether a step is configured correctly or not.
  - Currently, there's no simple way to see a step's inputs and outputs.
    - Add some UI for this.
  - Currently, reordering steps has some surprising side effects.
    - Instead of invalidating steps after reordering them, validate them at display time and warn the user.

Test Plan:
{F133679}
{F133680}

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley, chad

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8599
2014-03-25 16:02:34 -07:00
epriestley
5b74fa0a75 Make all build steps support variables
Summary: Ref T1049. This generally simplifies things. The steps which don't support variables generally don't make sense to support varaibles anyway.

Test Plan: Edited some steps.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8588
2014-03-25 16:02:07 -07:00
epriestley
150a3adf2c Minor UI improvements for Harbormaster
Summary: Ref T1049. Makes some minor UI tweaks.

Test Plan: Looked at UI.

Reviewers: btrahan

Reviewed By: btrahan

Subscribers: epriestley

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D8587
2014-03-25 13:59:43 -07:00
epriestley
a2a4f4b3da Fix validation of Harbormaster HTTP methods
Summary: Precedence here was mucked up.

Test Plan: Plan with no explicit "method" now defaults to POST correctly.

Reviewers: dctrwatson, btrahan

Reviewed By: btrahan

Subscribers: epriestley

Differential Revision: https://secure.phabricator.com/D8559
2014-03-18 12:05:14 -07:00
William R. Otte
29436dfe37 Added 'method' field to the HTTP request build step.
Summary:
This revision adds a 'method' field to the HTTP request harbormaster build step.  This allows the user to specify GET, POST, DELETE, and PUT (limited by the underlying wrapper phabricator uses for HTTP requests).  I'm not sure how much sense PUT makes, but oh well.

Existing plans shouldn't break, as if this field is an empty string, we default to POST, which is the old behavior.

Fixes T4604

Test Plan: 1) Verified that the empty string does, in fact, issue a POST request.  Changed the method to be GET and observed that the problem described in T4604 is resolved.

Reviewers: #blessed_reviewers, epriestley

Reviewed By: #blessed_reviewers, epriestley

Subscribers: aran, epriestley

Maniphest Tasks: T4604

Differential Revision: https://secure.phabricator.com/D8520
2014-03-13 15:51:05 -07:00
epriestley
458a28eed3 Truncate logSource in Harbormaster to the database column limit
Summary: This can be a command, which might be arbitrarily long, but the column is VARCHAR(255).

Test Plan: `grep`

Reviewers: dctrwatson, btrahan

Reviewed By: btrahan

CC: aran

Differential Revision: https://secure.phabricator.com/D8446
2014-03-07 17:43:46 -08:00
epriestley
70b008d18d Add test coverage that our definition of BMP agrees with MySQL
Summary:
Ref T1191. Test that MySQL's rules match those of `phutil_is_utf8_with_only_bmp_characters()`:

  - Build a string with //every// character that we consider to be a BMP character.
  - Write it into MySQL.
  - Read it back out.
  - Make sure MySQL didn't truncate it.

Test Plan: Ran unit test. This test runs pretty quickly (50ms), the string with every character isn't all that enormous.

Reviewers: btrahan, arice

Reviewed By: arice

CC: chad, arice, aran

Maniphest Tasks: T1191

Differential Revision: https://secure.phabricator.com/D8314
2014-02-23 16:20:38 -08:00
epriestley
21de2b1a0c Make Projects a PhabricatorSubscribableInterface, but with restricted defaults
Summary:
Ref T4379. I want project subscriptions to work like this (yell if this seems whacky, since it makes subscriptions mean somethign a little different for projects than they do for other objects):

  - You can only subscribe to a project if you're a project member.
  - When you're added as a member, you're added as a subscriber.
  - When you're removed as a member, you're removed as a subscriber.
  - While you're a member, you can optionally unsubscribe.

From a UI perspective:

  - We don't show the subscriber list, since it's going to be some uninteresting subset of the member list.
  - We don't show CC transactions in history, since they're an uninteresting near-approximation of the membership transactions.
  - You only see the subscription controls if you're a member.

To do this, I've augmented `PhabricatorSubscribableInterface` with two new methods. It would be nice if we were on PHP 5.4+ and could just use traits for this, but we should get data about version usage before we think about this. For now, copy/paste the default implementations into every implementing class.

Then, I implemented the interface in `PhabricatorProject` but with alternate defaults.

Test Plan:
  - Used the normal interaction on existing objects.
  - This has no actual effect on projects, verified no subscription stuff mysteriously appeared.
  - Hit the new error case by fiddling with the UI.

Reviewers: btrahan

Reviewed By: btrahan

CC: chad, aran

Maniphest Tasks: T4379

Differential Revision: https://secure.phabricator.com/D8165
2014-02-10 14:29:17 -08:00
epriestley
96dd530c44 Distinguish between "Remote URI" and "Clone URI" in Repositories
Summary:
Hosted repositories have muddied this distinction somewhat. In some cases, we only want to use the real remote URI, and the call is only relevant for imported repositories.

In other cases, we want the URI we'd plug into `git clone`.

Move this logic into `PhabricatorRepository` and make the distinction more clear.

Test Plan: Viewed SVN, Git, and Mercurial hosted and remote repositories, all the URIs looked reasonable.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran, dctrwatson

Differential Revision: https://secure.phabricator.com/D8096
2014-01-30 11:41:21 -08:00
James Rhodes
51c4f697f9 Delete artifacts when restarting build
Summary: Fixes T4336.  This updates the build engine to delete all artifacts when targets are being deleted.  This prevents conflicts when builds are restarted.

Test Plan: Restarted a build that had a lease host step and it didn't crash.

Reviewers: epriestley, #blessed_reviewers

Reviewed By: epriestley

CC: Korvin, epriestley, aran

Maniphest Tasks: T4336

Differential Revision: https://secure.phabricator.com/D8092
2014-01-28 20:17:36 -08:00
epriestley
996930da2a Improve several exception behaviors for Harbormaster workers
Summary:
Ref T2015. Several fixes:

  - `checkForCancellation()` no longer exists, and isn't relevant for resumable stops. Throw it away for now.
  - Fix an issue where a build could pass even if the final step failed.
  - `phlog()` exceptions so they show up in `bin/harbormaster` and the daemon logs.
  - Write an exception log if a step fails.
  - Add a "throw an exception" step to debug this stuff more easily.

Test Plan:
  - Grepped for `checkForCancellation()`.
  - Ran a failing build where the final step caused the failure.
  - Observed `phlog()` in `bin/harbormaster` output.
  - Observed log in web UI:

{F101168}

Reviewers: btrahan, hach-que

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2015

Differential Revision: https://secure.phabricator.com/D7935
2014-01-13 12:21:49 -08:00
epriestley
5502fca5f4 Make Drydock blueprint create workflow somewhat more standard
Summary: Ref T2015. This workflow is a little weird (runs in a dialog, no edit-before-create step, lots of internal classnames). Make it a little more standard.

Test Plan: See screenshots.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2015

Differential Revision: https://secure.phabricator.com/D7908
2014-01-08 14:12:27 -08:00
epriestley
46139bd1f6 Allow entire buildables to restart/stop/resume
Summary: Ref T1049. Creates convenience actions at the Buildable level to stop, resume, or restart all builds.

Test Plan:

  - Stopped all builds.
  - Resumed all builds.
  - Restarted all builds.

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7899
2014-01-06 14:12:15 -08:00
epriestley
b952b6f619 Improve restart/stop/resume UI
Summary:
Ref T1049. Improves the UI:

  - Pending commands, like "stopping", are shown separately from the current status.
  - Pending commands are shown on the list view.
  - Builds can be restarted, stopped and resumed from the list view.
  - Add a missing crumb.

Test Plan:
{F99022}

{F99023}

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T1049

Differential Revision: https://secure.phabricator.com/D7898
2014-01-06 14:12:05 -08:00