New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add scheduled event support closes #147 #192

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@johannesboyne
Contributor

johannesboyne commented Jan 29, 2016

I'll continue later and merge the master and than it closes #147 ;-)

@johannesboyne johannesboyne changed the title from Add scheduled event support to Add scheduled event support closes #147 Jan 29, 2016

johannesboyne added some commits Jan 29, 2016

@johannesboyne

This comment has been minimized.

Show comment
Hide comment
@johannesboyne

johannesboyne Jan 29, 2016

Contributor

@tj or @mthenw I've finished the implementation but what are your thoughts on deleting scheduled events? For now it is not implemented, just because I didn't want to query the connected CloudWatchEvents each time since most of the time there are no...

Contributor

johannesboyne commented Jan 29, 2016

@tj or @mthenw I've finished the implementation but what are your thoughts on deleting scheduled events? For now it is not implemented, just because I didn't want to query the connected CloudWatchEvents each time since most of the time there are no...

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Jan 29, 2016

Member

hmm good point, I guess ideally it's more of a terraform thing since it's super CRUD-like, I'll review in a bit!

Member

tj commented Jan 29, 2016

hmm good point, I guess ideally it's more of a terraform thing since it's super CRUD-like, I'll review in a bit!

_ "github.com/apex/apex/cmd/apex/logs"
_ "github.com/apex/apex/cmd/apex/docs"
_ "github.com/apex/apex/cmd/apex/metrics"
_ "github.com/apex/apex/cmd/apex/rollback"

This comment has been minimized.

@tj

tj Jan 29, 2016

Member

wonder if we can impose this order in cobra some other way hahaha _ + gofmt is annoying there

@tj

tj Jan 29, 2016

Member

wonder if we can impose this order in cobra some other way hahaha _ + gofmt is annoying there

Show outdated Hide outdated function/function.go
Show outdated Hide outdated function/function.go
Show outdated Hide outdated plugins/cron/cron.go
Show outdated Hide outdated hooks/hooks.go
Show outdated Hide outdated plugins/hooks/hooks.go
Show outdated Hide outdated sources/cron/cron.go
Show outdated Hide outdated sources/cron/cron.go
Show outdated Hide outdated plugins/cron/cron.go
func (c *Cron) connect(ruleName string) error {
res, err := c.CloudWatchService.PutTargets(&cloudwatchevents.PutTargetsInput{
Rule: aws.String(ruleName),

This comment has been minimized.

@tj

tj Jan 29, 2016

Member

I'll have to read up on this api a bit, seems crazy that we need three api calls just to add a little cron thing

@tj

tj Jan 29, 2016

Member

I'll have to read up on this api a bit, seems crazy that we need three api calls just to add a little cron thing

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Jan 29, 2016

Member

sucks TF doesn't have cloudwatchevents, sounds like they're on it hashicorp/terraform#4826, seems like a super terraform-y thing though after seeing the API calls. I'd like to stay away from cross into that zone too much, I just use TF for managing my sources right now. Anyone else have thoughts there? I can still see the value in having things with nice defaults like this out of the box, but I want to approach that as a layer on top of TF

Member

tj commented Jan 29, 2016

sucks TF doesn't have cloudwatchevents, sounds like they're on it hashicorp/terraform#4826, seems like a super terraform-y thing though after seeing the API calls. I'd like to stay away from cross into that zone too much, I just use TF for managing my sources right now. Anyone else have thoughts there? I can still see the value in having things with nice defaults like this out of the box, but I want to approach that as a layer on top of TF

@johannesboyne

This comment has been minimized.

Show comment
Hide comment
@johannesboyne

johannesboyne Jan 30, 2016

Contributor

Mhm... as a TF "fan" myself I'm with you... just don't want to wait for it, neither do I want to implement all of the CloudWatchEvents stuff for TF.

Contributor

johannesboyne commented Jan 30, 2016

Mhm... as a TF "fan" myself I'm with you... just don't want to wait for it, neither do I want to implement all of the CloudWatchEvents stuff for TF.

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Jan 30, 2016

Member

I don't know if it's smart for us to throw too much CRUD stuff in Apex though, it'll be awkwardly split between the two. In an ideal world we'd have it all but that's basically just Terraform. The main reason I started Apex was that Lambda function management was really bad in TF, but I don't think that will change haha it's not suited for repeated deploys or workflow related stuff

Member

tj commented Jan 30, 2016

I don't know if it's smart for us to throw too much CRUD stuff in Apex though, it'll be awkwardly split between the two. In an ideal world we'd have it all but that's basically just Terraform. The main reason I started Apex was that Lambda function management was really bad in TF, but I don't think that will change haha it's not suited for repeated deploys or workflow related stuff

@radeksimko

This comment has been minimized.

Show comment
Hide comment
@radeksimko

radeksimko Feb 5, 2016

Hi,
FYI I just finished the implementation of CloudWatch Events (both rules & targets) in hashicorp/terraform#4986

I'd be more than happy if you tested these resources before they reach upstream. 😉

This PR may also be interesting for apex: hashicorp/terraform#4826

radeksimko commented Feb 5, 2016

Hi,
FYI I just finished the implementation of CloudWatch Events (both rules & targets) in hashicorp/terraform#4986

I'd be more than happy if you tested these resources before they reach upstream. 😉

This PR may also be interesting for apex: hashicorp/terraform#4826

@tj

This comment has been minimized.

Show comment
Hide comment
@tj

tj Feb 5, 2016

Member

@radeksimko nice! thanks. Thanks as well @johannesboyne, I just think we might be safer to stick with the workflow side for now

Member

tj commented Feb 5, 2016

@radeksimko nice! thanks. Thanks as well @johannesboyne, I just think we might be safer to stick with the workflow side for now

@tj tj closed this Feb 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment