Skip to content
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

Added RemoveFunc, PauseFunc and ResumeFunc #15

Closed
wants to merge 17 commits into from
Closed

Conversation

@elgs
Copy link

@elgs elgs commented Aug 6, 2014

With some test code:

package main

import (
    "fmt"
    "cron"
    "time"
)

func main() {
    c := cron.New()
    j1, _ := c.AddFunc("* * * * * *", func() { fmt.Println("1") })
    j2, _ := c.AddFunc("* * * * * *", func() { fmt.Println("2") })
    j3, _ := c.AddFunc("* * * * * *", func() { fmt.Println("3") })
    j4, _ := c.AddFunc("* * * * * *", func() { fmt.Println("4") })
    fmt.Println(j1, j2, j3, j4)
    c.RemoveFunc(j2)
    fmt.Println("j2 removed")

    c.Start()

    time.Sleep(time.Second * 5)
    c.RemoveFunc(j1)
    fmt.Println("j1 removed")

    time.Sleep(time.Second * 5)
    c.PauseFunc(j4)
    fmt.Println("j4 paused")

    time.Sleep(time.Second * 5)
    c.ResumeFunc(j4)
    fmt.Println("j4 resumed")
    select {}
}

And the output of the code above:

1 2 3 4
j2 removed
1
3
4
1
3
4
1
3
4
1
3
4
1
3
4
j1 removed
3
4
3
4
3
4
3
4
3
4
j4 paused
3
3
3
3
3
j4 resumed
3
4
3
4
3
4
3
4
3
4

I'm sorry I don't know how to exclude the .gitignore from the pull request, please just ignore it.

@elgs elgs changed the title Added RemoveFunc Added RemoveFunc, PauseFunc and ResumeFunc Aug 6, 2014
@ahall
Copy link

@ahall ahall commented Aug 18, 2014

Any chance of getting this reviewed and merged @elgs @robfig ?

@elgs
Copy link
Author

@elgs elgs commented Aug 18, 2014

Thanks @ahall.
@robfig, although in the commit log, I mentioned "changed Entry from array to map.", it is actually still array/slice. Initially, I tried to change the Entry from array/slice to map, finally I found it was not necessary. Putting an Id in the Entry struct is enough for tracking them. Actually the modification is minimum. I appreciate if you take a look at the changes and consider to merge the changes.

Thanks,
Elgs

@elgs
Copy link
Author

@elgs elgs commented Aug 18, 2014

@ahall, before @robfig responds, alternatively, you may also use go get -u github.com/elgs/cron for a temporary solution.

@ahall
Copy link

@ahall ahall commented Aug 18, 2014

Yeah only thing I see wrong with this is might be worth rebasing this into a single commit instead of multiple commits.

@elgs
Copy link
Author

@elgs elgs commented Aug 18, 2014

@ahall, that would be great. However, I really need to be educated how to use these advanced features with Github.

@ahall
Copy link

@ahall ahall commented Aug 26, 2014

@robfig Have you had any chance to look at this?

@ahall
Copy link

@ahall ahall commented Sep 16, 2014

Ping @robfig

@SchumacherFM
Copy link

@SchumacherFM SchumacherFM commented Nov 14, 2014

What's the status?

@robfig
Copy link
Owner

@robfig robfig commented Dec 3, 2014

I think offering Remove is a simpler solution rather than having the concept of Pause and Resume (which is no different from Add and Remove I think).

Does this meet your needs?
a35e5c4

@elgs
Copy link
Author

@elgs elgs commented Dec 3, 2014

Right, Remove is needed to make it really useful. However, I'm building my task scheduler. Pause and Resume make the scheduler a lot easier to use.

@robfig
Copy link
Owner

@robfig robfig commented Dec 21, 2014

What is the difference in behavior between Remove() and Add() compared to Pause() and Resume()?

@elgs
Copy link
Author

@elgs elgs commented Dec 22, 2014

The difference between Add() and Resume() is that for Add(), you have to pass in the the job function itself again if you want to "resume" it, Resume() only requires an integer reference. I'm working on a job scheduler, it's easier for me to store the job instance references somewhere like in the database than to store the function. However, I understand without Pause() and Resume(), I can do a function to integer mapping to accomplish the same goal. This just makes my client code more straightforward.

elgs added 4 commits Jul 2, 2015
…t doesn't have a chance to be updated, so effective remains outdated, causing Next being called multiple times to catch up.
@yanzay
Copy link

@yanzay yanzay commented Apr 5, 2016

@robfig
Any chance to get it merged?

@elgs
Copy link
Author

@elgs elgs commented Apr 5, 2016

@yanzay I see less chance to get it merged now, as both branches have made incompatible changes. Somehow I agreed with @robfig that Pause and Resume are not absolutely necessary as there's actually not much difference to Remove it and re-Add it back. However, I think Remove should be provided at least, which is still missing from @robfig's branch. @yanzay please feel free to use my branch which has Remove, Pause and Resume implemented and I have been using it in my projects and it's proven to work pretty well.

@robfig
Copy link
Owner

@robfig robfig commented Aug 9, 2016

Yeah.. I'm hesitant to make large, non-backwards compatible changes that I have to maintain in the future. Feel free to use elgs !

@robfig robfig closed this Aug 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants