Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Add support for Cron Triggers #1592

Merged
merged 4 commits into from
Oct 23, 2020

Conversation

ObsidianMinor
Copy link
Contributor

  • This also refactors deployments to accept multiple deploy targets so you
    aren't limited to zoned, zoneless, or scheduled for one script.

  • It also removes the constraint that only route or routes can be used.
    If both are specified, routes will have route inserted at the beginning

This fixes #1574

}
}
#[derive(Debug, PartialEq, Clone)]
pub enum DeployTarget {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ashleymichal So it turns out removing all type information by using a boxed trait object makes testing and checks in wrangler dev really hard so I decided to just use an enum instead to simplify a lot of stuff...

@ObsidianMinor ObsidianMinor force-pushed the feat/cron-triggers branch 2 times, most recently from 4ecdc92 to bcd04d3 Compare October 10, 2020 00:22
@ashleymichal
Copy link
Contributor

can you please address the clippy issue?

 * This also refactors deployments to accept multiple deploy targets so you
   aren't limited to zoned, zoneless, or scheduled for one script.

 * It also removes the constraint that only route or routes can be used.
   If both are specified, routes will have route inserted at the beginning
Copy link
Contributor

@ashleymichal ashleymichal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for future PRs, it would be helpful to separate the refactors and feature work into different commits.

let's tidy up the output and tests. also have a look to see if there's changes needed to the readme.

src/commands/publish.rs Outdated Show resolved Hide resolved
src/deploy/mod.rs Outdated Show resolved Hide resolved
src/deploy/mod.rs Outdated Show resolved Hide resolved
src/deploy/zoned.rs Outdated Show resolved Hide resolved
src/deploy/zoneless.rs Show resolved Hide resolved
src/settings/toml/tests/deployments.rs Outdated Show resolved Hide resolved
src/settings/toml/tests/deployments.rs Outdated Show resolved Hide resolved
src/settings/toml/tests/deployments.rs Outdated Show resolved Hide resolved
src/settings/toml/tests/deployments.rs Outdated Show resolved Hide resolved
src/settings/toml/tests/deployments.rs Show resolved Hide resolved
@nataliescottdavidson
Copy link
Contributor

I checked this out, added crons = ["****"] to my wrangler.toml, and published my worker. It doesn't show up in the cron triggers view on that worker in the dashboard, though. Am I getting the usage wrong?

@ObsidianMinor
Copy link
Contributor Author

@nataliescottdavidson It needs to be added under a triggers table like this:

name = "wrangled-scheduled"
type = "javascript"
account_id = "blah"

[triggers]
crons = ["* * * * *"]

@ashleymichal ashleymichal merged commit 86be5e7 into cloudflare:master Oct 23, 2020
Copy link
Contributor

@ashleymichal ashleymichal left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks mucho!

name: target.name.clone(),
urls,
schedules,
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

excellent

src/deploy/zoneless.rs Show resolved Hide resolved
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mini RFC: Cron Triggers
3 participants