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

Add crontab timer function to run periodic job #6

Merged
merged 4 commits into from
Feb 9, 2021
Merged

Add crontab timer function to run periodic job #6

merged 4 commits into from
Feb 9, 2021

Conversation

lqhuang
Copy link
Collaborator

@lqhuang lqhuang commented Jan 19, 2021

Note: Before submitting this pull request, please review our contributing
guidelines
.

Description

I'm using mode as base framework to build some applications, but I found missing some features about Service.timer and Service.task.

So I try to add a crontab based timer into mode, and the major implementation is ported from faust code base.

It won't be better you could help to review this PR. If there are some problems, I can follow your instruction to improve them.

Regards.

Lanqing


Please describe your pull request.

NOTE: All patches should be made against master, not a maintenance branch like
3.1, 2.5, etc. That is unless the bug is already fixed in master, but not in
that version series.

If it fixes a bug or resolves a feature request,
be sure to link to that issue via (Fixes #4412) for example.

@max-k
Copy link

max-k commented Feb 7, 2021

IMO, porting back crontab from Faust to Mode is a good idea but if we do that, we'll also have to update Faust to make it consume crontab functionality from here instead of maintaining two similar features in two different projects.

Unfortunately, @patkivikram doesn't seem willing to accept merge requests on Faust project.

We opened a very simple MR with a teammate before Christmas and we still didn't get any comment at this time.

Moreover, this project lack of a communication system like Gitter so it's a bit difficult to open a discussion.

@patkivikram : Could you give us your opinion about that ? Thank you very much.

@patkivikram
Copy link

@max-k which PR are you referring to in faust-streaming? We are accepting and merging PR's on a regular basis

@lqhuang
Copy link
Collaborator Author

lqhuang commented Feb 9, 2021

OK, then I'm willing to do the job that update codes in faust after PR has been merged and released a new version.

@max-k
Copy link

max-k commented Feb 9, 2021

@patkivikram Hi. Thank you for your answer. I talk about This one : faust-streaming/faust#66
@lqhuang Thank you very much. It will be great if you can do that.

@max-k max-k merged commit ce20621 into faust-streaming:master Feb 9, 2021
@lqhuang
Copy link
Collaborator Author

lqhuang commented Feb 19, 2021

@max-k Thank you for merging my PR :) Future more, when would a new version be released? That will be helpful for me to update faust codes.

@max-k
Copy link

max-k commented Feb 19, 2021

Hi @lqhuang
I'll try to automate the release process during next week if I have enough time.
Otherwise I'll do a manual release at the end of the week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants