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

Month index (0-11) in cron expressions: Deliberated or Mistake #435

Closed
kb05 opened this issue Jan 11, 2021 · 10 comments
Closed

Month index (0-11) in cron expressions: Deliberated or Mistake #435

kb05 opened this issue Jan 11, 2021 · 10 comments

Comments

@kb05
Copy link

kb05 commented Jan 11, 2021

I'm submitting a...


[ ] Regression 
[x] Bug report
[ ] Feature request
[x] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

The problem is related with the month index in cron expressions, in linux cron jobs the the month has a range between 1-12, being January the 1 and December the 12. But the @nestjs/schedule library seem like uses another format in which January is 0 and December is 11.

Is this deliberate or is it a mistake?

Current behavior

@Cron('0 12 11 1 *')

Configure a job that will be executed "At 12:00 on day-of-month 11 in February." instead january.

Expected behavior

The job will be executed in january.

Minimal reproduction of the problem with instructions

Configure a service that use the next cron:

@Cron('0 12 11 1 *', {
        name: 'test',
})
async testCron(){
        console.log('hi', new Date())
}

What is the motivation / use case for changing the behavior?

I was trying to make a job that would run every 3 months (before quarter ends), and I couldn't get it to work because in january, for example, I was using the index 1 instead 0.

Environment

# Dockerfile
ARG NODE_VERSION=14.15.1

FROM node:${NODE_VERSION}

ARG NESTJS_VERSION=7.5

WORKDIR /usr/src/project

# Install the required tools
RUN apt update;

RUN apt-get install nano;

RUN npm install -g nodemon

RUN npm install -g @nestjs/cli@${NESTJS_VERSION};

The objective of the issue is to check if this is the correct functioning, and if so, to document it.

@kamilmysliwiec
Copy link
Member

@kb05
Copy link
Author

kb05 commented Jan 11, 2021

Hi @kamilmysliwiec.

In the link that you passed me appears as a range of values 1-12, and I'm reporting that, that the library uses 0 to 11.

image

@jkrajinovic
Copy link

jkrajinovic commented Sep 3, 2022

@kb05 You are right but I guess its not going to change since schedule depends on cron library which has
wrong range for month.

time js

This should be written in the doc.

@kamilmysliwiec the URL you have sent is wrong.

@micalevisk
Copy link
Member

has wrong range for month.

isn't wrong, it's just how the month part of Date is encoded (0-based)

image

@micalevisk
Copy link
Member

micalevisk commented Sep 3, 2022

I suggest reporting this to node-cron side instead. I can't tell if they're translating those 1-12 to 0-11 or not.

On NestJS docs there is a mention tp that package, so I guess there's nothing to do on our side.

@jkrajinovic
Copy link

jkrajinovic commented Sep 3, 2022

Thanks @micalevisk

Sure, are we talking about same cron lib since:
@kamilmysliwiec sent

https://github.com/node-cron/node-cron

but

@nestjs/schedule has dependency on

@micalevisk
Copy link
Member

oh, then we should update the docs. PRs are more than welcomed

https://github.com/kelektiv/node-cron#cron-ranges

@jkrajinovic
Copy link

Sure, what repo?

@micalevisk
Copy link
Member

micalevisk commented Sep 3, 2022

image

https://docs.nestjs.com/techniques/task-scheduling

Change that node-cron to cron. The hyperlink is correct

@jkrajinovic
Copy link

jkrajinovic commented Sep 3, 2022

nestjs/docs.nestjs.com#2452

Hope the PR is valid.

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

No branches or pull requests

4 participants