Add bin/trigger, for testing event triggers
ClosedPublic
Actions

Authored by epriestley on Jan 20 2015, 12:39 AM.

Details

Reviewers

btrahan

Maniphest Tasks

T6881: Implement subscription cancellation, past-due callbacks, and Merchant review of invoices in Phortune

Commits

Restricted Diffusion Commit
rP934df0e735f5: Add bin/trigger, for testing event triggers

Summary

Ref T6881. This makes it easier to fire a trigger and make sure it works properly. You can use the --now flag to travel through time, and test scheduling conditions with --last and --next. It will tell you when the trigger would reschedule.

Better than waiting 24 hours to see if things work.

Test Plan

Fired some backups, got useful output which made me think my code probably works correctly.

Diff Detail

Repository

rP Phabricator

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

epriestley updated this revision to Diff 27482.Jan 20 2015, 12:39 AM

epriestley retitled this revision from to Add bin/trigger, for testing event triggers.

epriestley updated this object.

epriestley edited the test plan for this revision. (Show Details)

epriestley added a reviewer: btrahan.

epriestley added a task: T6881: Implement subscription cancellation, past-due callbacks, and Merchant review of invoices in Phortune.

Herald added a subscriber: epriestley. · View Herald TranscriptJan 20 2015, 12:39 AM

btrahan accepted this revision.Jan 20 2015, 6:24 PM

btrahan edited edge metadata.

btrahan added inline comments.

src/infrastructure/daemon/workers/management/PhabricatorWorkerTriggerManagementWorkflow.php
45–56	Maybe you want to use the fancy phutil time stuff? I tend to get tripped up on any "time" parameter in CLI tools.

This revision is now accepted and ready to land.Jan 20 2015, 6:24 PM

epriestley added inline comments.Jan 20 2015, 7:30 PM

src/infrastructure/daemon/workers/management/PhabricatorWorkerTriggerManagementWorkflow.php
45–56	Yeah, the time locale stuff can get murky pretty quickly. The fancy phutil time stuff takes a viewer and we don't have a real viewer (specifically, we don't have a viewer with meaningful timezone preferences), which is why I'm not using it. Possibly I should and just pass a fake viewer. Generally, my "solution" for time stuff is to print the time out after we parse it so you can see what we ended up with, although `phabricator_datetime()` doesn't include timezone information so this might be limited in effect. A better approach might be to use some CLI date printing function which picks a longer-form, explicit-timezone format like "YYYY-MM-DD HH:MM:SS ZZZ". The actual behavior of this parameter and the printed times is to use the server's configured default timezone. If you set it to "America/Los_Angeles", you should be able to input times in local notation and get local outputs out. This is the same behavior you'd get by using the other parsing stuff with a fake/omnipotent viewer. However, in the Phacility cluster, the timezone is UTC, so `bin/trigger fire --now X` can mean something different on your local machine vs in the cluster. If this ends up being confusing, we could change the parsing to support or require an explicit timezone, e.g. `bin/trigger fire --timezone America/Los_Angeles --now X`, although that's pretty cumbersome. In all cases, you can specify dates and times in long form with a timezone to get unambiguous parses, although obviously that's also cumbersome. You can also figure out an epoch timestamp and use it with `@timestamp` notation if that's easier (e.g., if you have an epoch value from a database or something, you can just put @ on the front and copy-paste it, without needing to convert to a human-readable time). For now, let's just keep an eye on it and see how confusing it is? We might end up not needing to interact with it from the CLI very much. If it sucks, we can just ramp up the explicitness of timezones until it feels like we're in a good place on the safety checks.