amckinley (Austin McKinley)Administrator
User

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Monday

  • Clear sailing ahead.

User Details

User Since
Feb 20 2011, 8:41 PM (330 w, 5 d)
Roles
Administrator
Availability
Busy Busy at E1541: Austin in Vegas until 7:00 AM.

Recent Activity

Yesterday

amckinley accepted D18152: Degrade more gracefully when ProfileMenu dashboards fail to render.

Wording nit.

Fri, Jun 23, 7:17 PM
amckinley accepted D18132: Let PhabricatorSearchCheckboxesField survive saved query data with mismatched types.
Fri, Jun 23, 7:13 PM

Wed, Jun 21

amckinley added a comment to T12857: Temporary directory fullness can cause daemon issues?.

I think we should have our crontabs in version control regardless of whether or not we add tmpreaper to them, so I'll make a task for that.

Wed, Jun 21, 7:26 PM · Diffusion, Ops, Daemons, Phacility
amckinley added a comment to T12857: Temporary directory fullness can cause daemon issues?.

This should do the trick. It runs off atime by default. We could just set the time period to several days if we wanted to. Alternatively, if the filenames for extremely long-running jobs are predictable, there's a --protect '<shell_pattern>' argument we could use to avoid cleaning up those files.

Wed, Jun 21, 6:04 PM · Diffusion, Ops, Daemons, Phacility
amckinley added a comment to T12857: Temporary directory fullness can cause daemon issues?.

Should we just add a crontab entry to clean /tmp to paper this over until we get it fixed for real?

Wed, Jun 21, 5:27 PM · Diffusion, Ops, Daemons, Phacility

Tue, Jun 20

amckinley accepted D18142: Let "<select />" EditEngine fields canonicalize saved defaults.
Tue, Jun 20, 11:47 PM

Mon, Jun 19

amckinley updated the task description for T12856: Evaluate various "infrastructure-as-code" products.
Mon, Jun 19, 7:55 PM · Ops, Phacility
amckinley added a comment to T12856: Evaluate various "infrastructure-as-code" products.

Just out of curiosity, why Salt is not a candidate? I think it is very comparable to the others.

Mon, Jun 19, 7:55 PM · Ops, Phacility
amckinley accepted D18135: Improve validation errors for changing task priorities.
Mon, Jun 19, 7:44 PM
amckinley accepted D18134: Allow numeric constants to act as aliases for task priorities in the web UI <select />.
Mon, Jun 19, 7:34 PM
amckinley added a comment to T12856: Evaluate various "infrastructure-as-code" products.

Terraform Review
Pros:

  • Simple declarative syntax
  • Intended to be invoked first with a dry run, which shows in detail all the resources that will be rebuilt or altered
  • Written in Python, which everyone knows is superior to Ruby in every way
  • Very focused on "immutable" infrastructure. Instead of manipulating resources in place, prefers to spin up new resources with the new config and spin down the old ones. For the stateful hosts, we might need to be clever about mounting and unmounting EBS volumes when instances get rebuilt.
Mon, Jun 19, 7:27 PM · Ops, Phacility
amckinley created T12856: Evaluate various "infrastructure-as-code" products.
Mon, Jun 19, 7:14 PM · Ops, Phacility
amckinley added a comment to T12124: Counterintuitive priority setting via maniphest.edit conduit call.

Ahhh, that makes more sense. I parsed that as saying "we could migrate the code" instead of the saved objects. If you want to add the glue code now and then assign it back to me after, I'll work on the migration in my Copious Free Time™.

Mon, Jun 19, 6:22 PM · Maniphest, Conduit, Bug Report
amckinley added a comment to T12124: Counterintuitive priority setting via maniphest.edit conduit call.

If you're happy long-term with accepting all three types, I'm ok with that as a permanent fix. I'm planning on working through this week, just with weird/inconsistent hours.

Mon, Jun 19, 6:10 PM · Maniphest, Conduit, Bug Report
amckinley added a comment to T12124: Counterintuitive priority setting via maniphest.edit conduit call.

Are saved queries/EditEngine forms things we could write migrations for? That might be even more brittle (for example, what to do about saved queries for priorities that no longer exist on the install), but accepting three different priority specifications of two different types seems a little too Postel's Principle for me.

Mon, Jun 19, 5:56 PM · Maniphest, Conduit, Bug Report

Fri, Jun 16

amckinley added a comment to T12847: A Pathway Towards Private Clusters.

what network level safeguard

Fri, Jun 16, 6:14 PM · Ops, Phacility
amckinley added a comment to T12847: A Pathway Towards Private Clusters.

To back up a bit, how much human touching are you envisioning for bring up a new private cluster? In my head I was thinking we'd at least have to manually deal with requesting the cert, but it looks like There's An API For That, so I think it's at least possible to make the whole thing work magically. It would be something like filling out the form triggers the creation of a new terraform (or whatever) config which then gets deployed.

Fri, Jun 16, 4:56 PM · Ops, Phacility
amckinley added a comment to T12847: A Pathway Towards Private Clusters.

That we have VPC <-> VPC VPNs instead of subnet <-> subnet VPNs?

Fri, Jun 16, 4:12 PM · Ops, Phacility
chad awarded T12847: A Pathway Towards Private Clusters a Baby Tequila token.
Fri, Jun 16, 4:32 AM · Ops, Phacility
amckinley added a comment to T12847: A Pathway Towards Private Clusters.

Okie dokie, I'll start evaluating.

Fri, Jun 16, 2:03 AM · Ops, Phacility
amckinley closed T12848: Currently at max EC2 instance launch count as Resolved.

All done.

Fri, Jun 16, 1:52 AM · Ops, Phacility
amckinley added a comment to T12848: Currently at max EC2 instance launch count.

AWS support case ID is 2243430271.

Fri, Jun 16, 1:48 AM · Ops, Phacility
amckinley created T12848: Currently at max EC2 instance launch count.
Fri, Jun 16, 1:46 AM · Ops, Phacility

Thu, Jun 15

amckinley added a comment to T12847: A Pathway Towards Private Clusters.

I'm concerned that this may turn a 2-month project into a 6-month project if we write something ourselves, or a 12-month project if we use Ansible, Chef or Puppet.

Thu, Jun 15, 11:49 PM · Ops, Phacility
amckinley added a comment to T12847: A Pathway Towards Private Clusters.

I'll also suggest that we should implement some kind of "infrastructure as code" tool before going too far down the road of setting this stuff up. I'd really like to get to the point where no one ever has to push buttons in the AWS console unless something is really broken. I don't have super strong feelings about Ansible vs chef vs puppet, but I'd recommend against CFEngine and SaltStack.

Thu, Jun 15, 10:24 PM · Ops, Phacility
amckinley added a comment to T12847: A Pathway Towards Private Clusters.

As far as requesting certs goes, our "Request Certificate" button in the AWS console will work correctly by sending an email to the domain's owner:

Thu, Jun 15, 9:35 PM · Ops, Phacility
amckinley edited the content of 2017 Week 23 (Early June).
Thu, Jun 15, 5:16 PM

Wed, Jun 14

amckinley added a comment to T12124: Counterintuitive priority setting via maniphest.edit conduit call.

@aurelijus, D18111 is landed now in HEAD if you'd like to test this. Let us know if you have any issues.

Wed, Jun 14, 10:10 PM · Maniphest, Conduit, Bug Report
amckinley added a comment to T12846: arc unit uses the config of the current installation when running tests.

Philosophically I'm happy with that. Just as an exercise though, I came up with:

Wed, Jun 14, 10:08 PM · Bug Report
amckinley committed rP8008ade9af46: Use keywords instead of ints to update task priority in ManiphestEditEngine (authored by amckinley).
Use keywords instead of ints to update task priority in ManiphestEditEngine
Wed, Jun 14, 9:43 PM
amckinley closed T12124: Counterintuitive priority setting via maniphest.edit conduit call as Resolved by committing rP8008ade9af46: Use keywords instead of ints to update task priority in ManiphestEditEngine.
Wed, Jun 14, 9:43 PM · Maniphest, Conduit, Bug Report
amckinley closed D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine by committing rP8008ade9af46: Use keywords instead of ints to update task priority in ManiphestEditEngine.
Wed, Jun 14, 9:43 PM
amckinley updated the diff for D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.
  • correct collation
Wed, Jun 14, 9:42 PM
amckinley added a comment to D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.

@epriestley can you sanity check my migration real quick before I land this?

Wed, Jun 14, 9:04 PM
amckinley added a comment to T12846: arc unit uses the config of the current installation when running tests.

That makes sense. Probably not much to be done for it (other than potentially doing a config override for maniphest.priorities prior to running ManiphestTaskTestCase).

Wed, Jun 14, 8:31 PM · Bug Report
amckinley updated the diff for D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.
  • change valiation, add migration
Wed, Jun 14, 8:30 PM
amckinley added a comment to D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.

As compelling as having status == 🐫🐪 would be, I'm also envisioning issues with parsing email/bot commands if we start allowing !, quote characters, etc. I'm going to change the validation to 1-64 alphanumeric characters that aren't purely numeric and leave it at that.

Wed, Jun 14, 7:56 PM
amckinley created T12846: arc unit uses the config of the current installation when running tests.
Wed, Jun 14, 7:51 PM · Bug Report
amckinley created T12845: Setting and removing a config for maniphest.priorities results in a null config instead of reverting to the default.
Wed, Jun 14, 7:41 PM · Bug Report
amckinley added a comment to D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.

And just to be clear, you're saying all three of these strings should be following the same validation rules, right?

Wed, Jun 14, 7:37 PM
amckinley added a comment to D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.

And I can't come up with any real reason not to let users use "🐐🐐🐫🐪" as their primary stored key in the database, so I think weakening the validation is otherwise fine.

Wed, Jun 14, 7:30 PM

Tue, Jun 13

amckinley added inline comments to D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.
Tue, Jun 13, 9:29 PM
amckinley added a comment to D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.

I couldn't figure out how to test the changes to these classes (but I could probably get email working enough to test email commands and herald actions):

Tue, Jun 13, 8:34 PM
amckinley added a comment to D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.

This is every callsite that I could find (by grepping for ManiphestTaskPriorityTransaction::TRANSACTIONTYPE). I heavily tested the situation where a user wants to edit a task that's had its priority removed from the config. Also, see attached screenshot of the new keyword validation code in action:

Tue, Jun 13, 8:14 PM
amckinley updated the diff for D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.
  • fixing more callsites
  • removing isNewObject() check in ManiphestTaskPriorityTransaction which was breaking assigning priorities to new tasks
Tue, Jun 13, 8:12 PM
amckinley updated the diff for D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.
  • added new constant for representing priority keywords that dont exist any longer
  • added validation for priority keywords thats identical to status constant validation (alphanumeric, 1-12 characters)
  • marked priority keywords as required
Tue, Jun 13, 7:44 PM

Mon, Jun 12

amckinley awarded T12798: Decommission cluster host `repo012` a Baby Tequila token.
Mon, Jun 12, 6:36 PM · Ops, Phacility
amckinley added a comment to D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.

I'll probably go down the road of making the narrowest fix: new magic keyword constant that ManiphestTaskPriority treats as a no-op, with a nice comment explaining the weird behavior.

Mon, Jun 12, 6:30 PM
amckinley added a comment to D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.

We need a new UserShotOwnFootDetectorDaemon. How hard could it be to enumerate all possible bad states and test for them?

Mon, Jun 12, 6:11 PM
amckinley added a project to E1541: Austin in Vegas: Phacility.
Mon, Jun 12, 6:04 PM · Phacility
amckinley created E1541: Austin in Vegas.
Mon, Jun 12, 6:03 PM · Phacility
amckinley added a comment to D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.

Yeah, I'm not a fan of the way I changed the "current status no longer exists" code. In general I'm starting to feel like this change is too big and already too magical. Are there other transactions that work similarly to how ManiphestPriorityTransactions would work after this diff? Where constants get turned into different representations of the same constants when they get serialized?

Mon, Jun 12, 5:37 PM

Fri, Jun 9

amckinley updated the diff for D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.
  • more callsites for priority changing
Fri, Jun 9, 11:31 PM
amckinley added a comment to D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.

There are a few other callsites that create task priority transactions that still need to be cleaned up. Hopefully nothing else sets task priorities without going through the transactions layer.

Fri, Jun 9, 10:58 PM
amckinley created D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.
Fri, Jun 9, 10:52 PM
amckinley added a revision to T12124: Counterintuitive priority setting via maniphest.edit conduit call: D18111: Use keywords instead of ints to update task priority in ManiphestEditEngine.
Fri, Jun 9, 10:52 PM · Maniphest, Conduit, Bug Report
amckinley added a comment to T12124: Counterintuitive priority setting via maniphest.edit conduit call.

With only those fields:

{
  "data": [
    {
      "name": "Open",
      "value": "open",
      "special": "default",
      "closed": false
    },
    {
      "name": "Resolved",
      "value": "resolved",
      "special": "closed",
      "closed": true
    },
    {
      "name": "Wontfix",
      "value": "wontfix",
      "closed": true
    },
    {
      "name": "Invalid",
      "value": "invalid",
      "closed": true
    },
    {
      "name": "Duplicate",
      "value": "duplicate",
      "special": "duplicate",
      "closed": true
    },
    {
      "name": "Spite",
      "value": "spite",
      "closed": true
    }
  ]
}
Fri, Jun 9, 7:44 PM · Maniphest, Conduit, Bug Report
amckinley added a comment to T12124: Counterintuitive priority setting via maniphest.edit conduit call.

Similar example for new maniphest.status.search. I'm happy to drop any of these fields if you think we won't need them (prefixes and suffixes leap to mind).

Fri, Jun 9, 7:36 PM · Maniphest, Conduit, Bug Report

Thu, Jun 8

amckinley accepted D18107: Add a retroactive migration to expand the `contentHash` field.

Assuming I correctly understand all the discussion in D18037, this looks good to me.

Thu, Jun 8, 2:18 PM

Wed, Jun 7

amckinley added a comment to T12804: Diffusion pre-redesign UI feedback (June 2017).

I'd like to see little forward and back arrows at the top of the page when viewing actual commit objects. This use case might be unusual, but I've needed to find "the last commit before DXXX" and "the first commit after DYYY" a few times. GitHub doesn't appear to offer this either, but it feels a little easier than just providing links to the parent commit.

Wed, Jun 7, 10:32 PM · Diffusion, Design & Planning
epriestley awarded D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates a Mountain of Wealth token.
Wed, Jun 7, 9:09 PM
amckinley committed rPfb9d036e574f: Show task duplicates as related objects in Maniphest and migrate old duplicates (authored by amckinley).
Show task duplicates as related objects in Maniphest and migrate old duplicates
Wed, Jun 7, 8:30 PM
amckinley closed D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates by committing rPfb9d036e574f: Show task duplicates as related objects in Maniphest and migrate old duplicates.
Wed, Jun 7, 8:30 PM
amckinley updated the diff for D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates.
  • remove global function
Wed, Jun 7, 8:25 PM
amckinley retitled D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates from Show task duplicates as related objects in Maniphest to Show task duplicates as related objects in Maniphest and migrate old duplicates.
Wed, Jun 7, 8:04 PM
amckinley updated the diff for D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates.
  • migrate both types
  • update to correctly handle both kinds of dupe logic
  • cleanup
Wed, Jun 7, 8:01 PM

Tue, Jun 6

amckinley added a comment to T12798: Decommission cluster host `repo012`.

Oh. My. God.

Tue, Jun 6, 3:49 PM · Ops, Phacility
amckinley added a comment to T12798: Decommission cluster host `repo012`.

(Slightly related: I killed the test-nat instance this morning since it doesn't help actually test the NAT).

Tue, Jun 6, 3:41 PM · Ops, Phacility
amckinley added a comment to T12798: Decommission cluster host `repo012`.

As far as pulling this state from EC2, it's really easy to fetch the list of hosts that have a given tag: http://docs.aws.amazon.com/cli/latest/reference/ec2/describe-instances.html

Tue, Jun 6, 12:09 AM · Ops, Phacility

Mon, Jun 5

amckinley added a comment to T11413: Support renaming Phacility instances.

Oh, I see what you're saying. Yeah, whatever logical naming scheme we come up with, we should block that pattern for display names.

Mon, Jun 5, 8:53 PM · Phacility
amckinley added a comment to T11413: Support renaming Phacility instances.

If we stop generating internal IDs from instance names, I think we should pick a generation scheme such that they can not ever collide

Mon, Jun 5, 7:49 PM · Phacility
amckinley added a comment to T11413: Support renaming Phacility instances.

FWIW, I'm heavily in favor of the "display rename" approach, and allocating logical ids like ErrantAnalyticalGiraffe. Breaking the coupling between instance display name and its backing resources is a band-aid that will only get more painful to remove after we launch private instances.

Mon, Jun 5, 7:00 PM · Phacility

Fri, Jun 2

amckinley accepted D18069: Fix an issue where Phriction moves to new locations would fail with a "content required" error.
Fri, Jun 2, 11:15 PM

Mon, May 29

amckinley updated the diff for D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates.
  • wrong type
Mon, May 29, 6:36 PM
amckinley added a comment to D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates.

Oh, derp, you're right. Edge went the wrong way.

Mon, May 29, 6:33 PM
amckinley added a comment to D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates.

Hmmm this code is definitely creating the new edges (and is definitely not writing any new transactions), but now the UI changes aren't actually showing any dupes.

Mon, May 29, 6:05 PM
amckinley updated the diff for D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates.
  • switch to using EdgeEditor and LiskMigrationIterator
Mon, May 29, 5:58 PM
amckinley added inline comments to D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates.
Mon, May 29, 4:30 PM
amckinley added a comment to D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates.

Tested the migration by reverting to a pre-D10427 revision, making a bunch of tasks with dupes, and then running the migration. Observed expected new rows in phabricator_maniphest.edge table.

Mon, May 29, 4:26 PM
amckinley updated the diff for D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates.
  • add migration for D10427-style task merges
Mon, May 29, 4:19 PM

Sun, May 28

amckinley created P2055 migration.php.
Sun, May 28, 10:51 PM

Sat, May 27

amckinley closed T12767: Viewing a badge details page should show users that have received it as Invalid.

Yep, that would be the page. I was expecting to see details ala "Related Objects" in Maniphest and failed to move my eyes sufficiently left.

Sat, May 27, 3:05 AM · Badges, Feature Request
amckinley created T12767: Viewing a badge details page should show users that have received it.
Sat, May 27, 2:58 AM · Badges, Feature Request
amckinley accepted D18038: Add needActiveDiffs to differential.createcomment method.

ohkay

Sat, May 27, 2:50 AM
amckinley added a comment to T12234: Add a "Duplicates" tab to the "Related Objects" section on task page.

From a technical standpoint, we did not record these relationships in a normalized way prior to D16196 (June 2016, about 7 months ago) -- they only existed in the transaction log, and we just wrote a "close", extra_data="as duplicate of X" transaction that can't be queried efficiently.

Sat, May 27, 1:17 AM · Customer Impact, Feature Request
amckinley created D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates.
Sat, May 27, 1:14 AM
amckinley added a revision to T12234: Add a "Duplicates" tab to the "Related Objects" section on task page: D18037: Show task duplicates as related objects in Maniphest and migrate old duplicates.
Sat, May 27, 1:14 AM · Customer Impact, Feature Request
amckinley claimed T12234: Add a "Duplicates" tab to the "Related Objects" section on task page.
Sat, May 27, 1:05 AM · Customer Impact, Feature Request

Fri, May 26

amckinley committed rP9d37ad3022bf: Add maniphest.priority.search method (authored by amckinley).
Add maniphest.priority.search method
Fri, May 26, 11:02 PM
amckinley closed D18035: Add maniphest.priority.search method by committing rP9d37ad3022bf: Add maniphest.priority.search method.
Fri, May 26, 11:02 PM
amckinley created D18035: Add maniphest.priority.search method.
Fri, May 26, 10:43 PM
amckinley added a comment to T12124: Counterintuitive priority setting via maniphest.edit conduit call.

Actual example output:

Fri, May 26, 10:30 PM · Maniphest, Conduit, Bug Report
amckinley added a comment to T12124: Counterintuitive priority setting via maniphest.edit conduit call.

So should the return value for priority.search look something like this?

Fri, May 26, 10:22 PM · Maniphest, Conduit, Bug Report
amckinley committed rP04fd93e51e03: Drop DifferentialDraft storage (authored by amckinley).
Drop DifferentialDraft storage
Fri, May 26, 8:59 PM
amckinley closed T12104: Clean up remnants of old Differential draft storage as Resolved by committing rP04fd93e51e03: Drop DifferentialDraft storage.
Fri, May 26, 8:59 PM · Infrastructure, Differential
amckinley closed D18034: Drop DifferentialDraft storage by committing rP04fd93e51e03: Drop DifferentialDraft storage.
Fri, May 26, 8:59 PM
amckinley created D18034: Drop DifferentialDraft storage.
Fri, May 26, 8:51 PM
amckinley added a revision to T12104: Clean up remnants of old Differential draft storage: D18034: Drop DifferentialDraft storage.
Fri, May 26, 8:51 PM · Infrastructure, Differential
amckinley claimed T12124: Counterintuitive priority setting via maniphest.edit conduit call.

It looks like what we really need is a new method like maniphest.querystatuses, called maniphest.querypriorities to avoid anyone trying to hardcode these values.

Fri, May 26, 8:43 PM · Maniphest, Conduit, Bug Report
amckinley claimed T12104: Clean up remnants of old Differential draft storage.

This looks pretty easy. No mentions of DifferentialDraft in the codebase other than the class itself and a single migration. Steps here (I think) are:

Fri, May 26, 8:25 PM · Infrastructure, Differential