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.

Sign up for GitHub

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

Jump to bottom

option to wait for callbacks completion on job.stop() #713

Closed
Tracked by #674
YannHulot opened this issue Oct 12, 2023 · 3 comments · Fixed by #894
Closed
Tracked by #674

option to wait for callbacks completion on job.stop() #713

YannHulot opened this issue Oct 12, 2023 · 3 comments · Fixed by #894
Labels
good first issue Good for newcomers hacktoberfest This issue is registered for Hacktoberfest! help wanted Extra attention is needed released status:ready Ready to start implementation type:feature New feature or feature improvement & requests

Comments

@YannHulot
Copy link

YannHulot commented Oct 12, 2023 •
edited by sheerlox
Loading

⭐ Suggestion

Do not stop the jobs if the onTick function is being executed. Wait for this function to resolve first and then, resolve job.stop()

💻 Use Cases

Basically I am trying to gracefully shut down my cron jobs when deploying a new version of my back-end.

That means that ideally, I want to call job.stop() on all the ongoing jobs, Then once they are done, I want to close the DB connection and so on and so forth.

The problem is that those jobs are mostly doing async stuff, so if I call a job.stop() and my onTick function is currently running, then, I would like to wait for the onTick function to resolve before the job.stop() function resolves itself so I don't close the DB connection before those jobs are done since they need it.

Related
@YannHulot YannHulot added the type:feature New feature or feature improvement & requests label Oct 12, 2023
@sheerlox sheerlox changed the title Let onTick function resolve before shutting down jobs when calling job.stop() option to wait for callbacks completion on job.stop() Oct 12, 2023
@sheerlox
Copy link
Collaborator

related to #556 since both features need to watch for callbacks completion

This was referenced Oct 12, 2023
Closed
Open
@sheerlox sheerlox added hacktoberfest This issue is registered for Hacktoberfest! status:ready Ready to start implementation help wanted Extra attention is needed good first issue Good for newcomers labels Oct 12, 2023
@Zamoca42 Zamoca42 mentioned this issue Sep 4, 2024
Merged
9 tasks
@rohith1222004
Copy link

I noticed that PR #894 is linked to this issue for supporting async handling and job status tracking. Is this feature close to completion, or would you still like help working on it?

intcreator added a commit that referenced this issue Dec 10, 2024
@Zamoca42 @intcreator
feat: support async handling and add CronJob status tracking (#894)
b58fb6b 
## Description
This PR improves async handling in the CronJob class and adds status
tracking functionality.

- Modified the `fireOnTick()` method to return a Promise for better
async callback handling
- Added an `isCallbackRunning` flag to track the running state of
CronJob instances
- Updated the test suite to use the new async behavior and track the
job's running state
- Added `waitForCompletion` functionality to the `job.stop()` method
- waits for running jobs to complete before executing the `onComplete`
callback

During test case writing, I encountered a type error with sinon.
To resolve this, added `sinon.restore()` to the `afterEach` block.

Reference:
https://stackoverflow.com/questions/73232999/sinon-cant-install-fake-timers-twice-on-the-same-global-object

<img width="811" alt="스크린샷 2024-09-03 오후 7 10 58"
src="https://github.com/user-attachments/assets/b87deee7-14b2-4407-8fea-a1cb469ef44b">

## Related Issue
Closes #713
Closes #556

## Motivation and Context
These changes allow the CronJob class to handle asynchronous callbacks
more effectively and provide a way to track the running state of jobs.

## How Has This Been Tested?
- Updated existing test suite to verify the new async behavior
- Adjusted test timeouts to use the `tickAsync` method
- Added new test cases to check for proper waiting of running callbacks
before stopping

## Types of changes
- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to change)

## Checklist:
- [x] My code follows the code style of this project.
- [x] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
- [x] I have added tests to cover my changes.
- [x] All new and existing tests passed.
- [ ] If my change introduces a breaking change, I have added a `!`
after the type/scope in the title (see the Conventional Commits
standard).

---------

Co-authored-by: Brandon der Blätter <intcreator@users.noreply.github.com>
@ncb000gt
Copy link
Member

🎉 This issue has been resolved in version 3.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

JosephVoid pushed a commit to JosephVoid/node-cron that referenced this issue Dec 14, 2024
@semantic-release-bot
Release v3.3.0 [skip ci]
f4e9e02 
## [3.3.0](kelektiv/node-cron@v3.2.1...v3.3.0) (2024-12-10)

### ✨ Features

* support async handling and add CronJob status tracking ([kelektiv#894](kelektiv#894)) ([b58fb6b](kelektiv@b58fb6b)), closes [kelektiv#713](kelektiv#713) [kelektiv#556](kelektiv#556)

### ⚙️ Continuous Integrations

* **action:** update github/codeql-action action to v3.27.2 ([kelektiv#912](kelektiv#912)) ([d11ba30](kelektiv@d11ba30))
* **action:** update github/codeql-action action to v3.27.5 ([kelektiv#917](kelektiv#917)) ([2a4035e](kelektiv@2a4035e))
* **action:** update step-security/harden-runner action to v2.10.2 ([kelektiv#920](kelektiv#920)) ([26a8f9f](kelektiv@26a8f9f))
* add pre-commit hook to lint and prettify ([kelektiv#911](kelektiv#911)) ([e1140d1](kelektiv@e1140d1)), closes [kelektiv#907](kelektiv#907)

### ♻️ Chores

* **deps:** lock file maintenance ([94465ae](kelektiv@94465ae))
* **deps:** lock file maintenance ([23d67a4](kelektiv@23d67a4))
* **deps:** lock file maintenance ([135fdf7](kelektiv@135fdf7))
* **deps:** lock file maintenance ([edcff3b](kelektiv@edcff3b))
* **deps:** pin dependency lint-staged to 15.2.10 ([kelektiv#916](kelektiv#916)) ([5cf24da](kelektiv@5cf24da))
* **deps:** update dependency [@commitlint](https://github.com/commitlint)/cli to v19.6.0 ([9d9ab94](kelektiv@9d9ab94))
* **deps:** update dependency [@types](https://github.com/types)/node to v20.17.7 ([9181b6a](kelektiv@9181b6a))
* **deps:** update dependency [@types](https://github.com/types)/node to v20.17.8 ([5899fc2](kelektiv@5899fc2))
* **deps:** update dependency [@types](https://github.com/types)/node to v20.17.9 ([ca5065a](kelektiv@ca5065a))
* **deps:** update dependency husky to v9.1.7 ([a960a29](kelektiv@a960a29))
* **deps:** update dependency typescript to v5.7.2 ([3447ff5](kelektiv@3447ff5))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers hacktoberfest This issue is registered for Hacktoberfest! help wanted Extra attention is needed released status:ready Ready to start implementation type:feature New feature or feature improvement & requests
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat: support async handling and add CronJob status tracking Zamoca42/node-cron
4 participants
@ncb000gt @sheerlox @YannHulot @rohith1222004