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

RecurrenceTrigger with iCalendar rfc5545 recurrence rules added. #153

Closed
wants to merge 4 commits into from

Conversation

elfogre
Copy link

@elfogre elfogre commented Jun 14, 2017

I have a need to use recurrence rules to define quartz jobs. I read https://jira.terracotta.org/jira/browse/QTZ-252 discussion about it and decide to implement over the solution suggested in the issue. I've also included tests for the new trigger. (Commit checked this time)

@zemian
Copy link
Contributor

zemian commented Jun 29, 2017

Thanks for the PR @elfogre. I have not used iCalendar myself, and glad to see that you able to make it work for Quartz. However making this as part of quartz core would require more maintenance, including bringing in third party library. I would leave this PR up and see how many users are voting for this before considering accepting.

Thanks,
Zemian

@ankurbhakta
Copy link

+1

9 similar comments
@RafaelTaranto
Copy link

+1

@AlexeyDeev
Copy link

+1

@kazemat
Copy link

kazemat commented Mar 14, 2018

+1

@atkawa7
Copy link

atkawa7 commented Apr 7, 2018

+1

@YingchunLi
Copy link

+1

@nilayjain
Copy link

+1

@atfire
Copy link

atfire commented Jun 5, 2018

+1

@neeryck
Copy link

neeryck commented Nov 9, 2018

+1

@raychin4563
Copy link

+1

@zemian
Copy link
Contributor

zemian commented Feb 14, 2019

Hi @elfogre

Your changes require a Contribution License Agreement (CLA) to be submitted.
You can find the instructions in http://www.quartz-scheduler.org/community/contribute.html

Can you please submit the CLA so we can consider merging in your changes?

@elfogre
Copy link
Author

elfogre commented Feb 15, 2019

CLA submitted. I hope this work helps you!

@zemian
Copy link
Contributor

zemian commented Feb 15, 2019

Thank you @elfogre ! The PR seems to be outdated. If you have time, please help resync it. I will try to look into these new code in the next few days!

@elfogre
Copy link
Author

elfogre commented Feb 18, 2019

Hi @zemian . I've updated the code to be mergeable. I dind't see any test failure in azure pipeline, but it looks it failed due to a timeout. I'm going to update libraries versions and make a commit to force new checks.

@zemian
Copy link
Contributor

zemian commented Feb 19, 2019

Thank you @elfogre !! I will review this and try to merge in.

@jreber
Copy link

jreber commented Mar 20, 2019

I have looked through -- and used -- the code in the PR, and the one thing I can't figure out is how to incorporate an arbitrary timezone into an rrule trigger. My server is set to UTC, so RecurrenceRuleTriggerImpl#getTimeAfter uses UTC -- but I want to create a trigger in a different, daylight saving time-aware time zone.

I see RecurrenceRuleScheduleBuilder has a commented-out method inTimeZone that seems like it does the trick -- except that it's commented out and relies on a method Biweekly method (Recurrence#setTimeZone) that doesn't seem to exist.

Am I missing anything here about Quartz/RRules -- say, Quartz being philosophically opposed to time zones, or there already being a way to specify a time zone in a trigger? Or is hard-coding TimeZone.getDefault() just an oversight? Is this feature -- specifying arbitrary time zones for an rrule trigger -- a feature you'd want in the Quartz code base?

@atkawa7
Copy link

atkawa7 commented Mar 21, 2019

@zemian PING

@zemian
Copy link
Contributor

zemian commented Mar 21, 2019

Hi, Sorry, I thought I would have some more time to put into the project when I first replied, but now I am busy again. I will come back to this later when time is allowed.

@athuljayaraj
Copy link

+1

@chrisdennis
Copy link
Contributor

chrisdennis commented Oct 4, 2019

To me this looks like it should be a separate module to core. Maybe that module could be within quartz itself as a 'exotic-triggers' module or in a separate repo under the Quartz project? I can imagine us setting up an 'exotic-triggers' repository where many of these kinds of things could live. Thoughts?

@chrisdennis
Copy link
Contributor

Closing in lieu of my comment above. If someone wants to reconstitute this PR as a proposal for a new project then we can reopen the discussion and review.

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.

None yet