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

Takes forever with invalid cron string #221

Closed
ramontayag opened this Issue Nov 16, 2016 · 6 comments

Comments

Projects
None yet
2 participants
@ramontayag

ramontayag commented Nov 16, 2016

Try this and it will run forever:

Rufus::Scheduler::CronLine.new("0 0 0 * *").next_time(Time.now)

This should raise an error right?

@jmettraux

This comment has been minimized.

Show comment
Hide comment
@jmettraux

jmettraux Nov 16, 2016

Owner

Hello,

what error should it raise?

Owner

jmettraux commented Nov 16, 2016

Hello,

what error should it raise?

@jmettraux jmettraux assigned ramontayag and jmettraux and unassigned ramontayag Nov 16, 2016

@ramontayag

This comment has been minimized.

Show comment
Hide comment
@ramontayag

ramontayag Nov 16, 2016

I'm thinking invalid cron syntax or something like that. I can work on
this.

On Wed, 16 Nov 2016 11:33 John Mettraux, notifications@github.com wrote:

What error should it raise?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#221 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABi9G5mtfOa6rwZe6ad8uRyGVla2BUTks5q-nmPgaJpZM4KzS1t
.

ramontayag commented Nov 16, 2016

I'm thinking invalid cron syntax or something like that. I can work on
this.

On Wed, 16 Nov 2016 11:33 John Mettraux, notifications@github.com wrote:

What error should it raise?


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#221 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABi9G5mtfOa6rwZe6ad8uRyGVla2BUTks5q-nmPgaJpZM4KzS1t
.

@jmettraux

This comment has been minimized.

Show comment
Hide comment
@jmettraux

jmettraux Nov 16, 2016

Owner

Actually, you are right.

Rufus::Scheduler::CronLine.new("0 0 0 * *")
  # and
Rufus::Scheduler::CronLine.new("* * 0 * *")

should raise an ArgumentError, since day 0 is not allowed. The error should be raised on CronLine#initialize, not on CronLine#next_time.

I will fix that for the next release, after I'm done with gh-220.

Thanks a lot for reporting that!

Owner

jmettraux commented Nov 16, 2016

Actually, you are right.

Rufus::Scheduler::CronLine.new("0 0 0 * *")
  # and
Rufus::Scheduler::CronLine.new("* * 0 * *")

should raise an ArgumentError, since day 0 is not allowed. The error should be raised on CronLine#initialize, not on CronLine#next_time.

I will fix that for the next release, after I'm done with gh-220.

Thanks a lot for reporting that!

@ramontayag

This comment has been minimized.

Show comment
Hide comment
@ramontayag

ramontayag Nov 16, 2016

Ok cool thanks ☺

On Wed, 16 Nov 2016 11:37 John Mettraux, notifications@github.com wrote:

Actually, you are right.

Rufus::Scheduler::CronLine.new("0 0 0 * _")

andRufus::Scheduler::CronLine.new("_ * 0 * *")

should raise an ArgumentError, since day 0 is not allowed. The error
should be raised on CronLine#initialize, not on CronLine#next_time.

I will fix that for the next release, after I'm done with gh-220
#220.

Thanks a lot for reporting that!


You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub
#221 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABi9CGJYcrZxMPVecEgt683RjnJiDwGks5q-nqQgaJpZM4KzS1t
.

ramontayag commented Nov 16, 2016

Ok cool thanks ☺

On Wed, 16 Nov 2016 11:37 John Mettraux, notifications@github.com wrote:

Actually, you are right.

Rufus::Scheduler::CronLine.new("0 0 0 * _")

andRufus::Scheduler::CronLine.new("_ * 0 * *")

should raise an ArgumentError, since day 0 is not allowed. The error
should be raised on CronLine#initialize, not on CronLine#next_time.

I will fix that for the next release, after I'm done with gh-220
#220.

Thanks a lot for reporting that!


You are receiving this because you were assigned.

Reply to this email directly, view it on GitHub
#221 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AABi9CGJYcrZxMPVecEgt683RjnJiDwGks5q-nqQgaJpZM4KzS1t
.

@jmettraux jmettraux closed this in 93e077b Nov 21, 2016

@jmettraux

This comment has been minimized.

Show comment
Hide comment
@jmettraux

jmettraux Nov 21, 2016

Owner

Hello Ramon,

I made sure "day 0" is avoided. Please have a look and tell me if you spot anything off.

Many thanks!

John

Owner

jmettraux commented Nov 21, 2016

Hello Ramon,

I made sure "day 0" is avoided. Please have a look and tell me if you spot anything off.

Many thanks!

John

@ramontayag

This comment has been minimized.

Show comment
Hide comment
@ramontayag

ramontayag Nov 22, 2016

Looks great. Thank you!

ramontayag commented Nov 22, 2016

Looks great. Thank you!

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