-
Notifications
You must be signed in to change notification settings - Fork 410
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
feat(repeatable): allow saving custom key #1824
Conversation
src/classes/repeat.ts
Outdated
jobId?: string; | ||
key?: string; | ||
}) { | ||
const checksum = key ? key : md5(`${name}${jobId || ''}${md5(namespace)}`); | ||
return `repeat:${checksum}:${nextMillis}`; | ||
// return `repeat:${jobId || ''}:${name}:${namespace}:${nextMillis}`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets remove these out commented code as well 👍
src/interfaces/repeat-options.ts
Outdated
@@ -10,6 +10,12 @@ export interface RepeatOptions extends Omit<ParserOptions, 'iterator'> { | |||
* A repeat pattern | |||
*/ | |||
pattern?: string; | |||
|
|||
/** | |||
* Custom repeatable key |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Complete with more information, like why is this needed:
"Custom repeatable key. This is the key that holds the "metadata" of a given repeatable job. This key is normally auto-generated but it is sometimes useful to specify a custom key for easier retrieval of repeatable jobs."
src/classes/repeat.ts
Outdated
); | ||
nextMillis: '', | ||
namespace: repeatJobKey, | ||
jobId: jobId || repeat.jobId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would recommend the use of a nullish coalescing operator jobId: jobId ?? repeat.jobId
src/classes/repeat.ts
Outdated
jobId?: string; | ||
key?: string; | ||
}) { | ||
const checksum = key ? key : md5(`${name}${jobId || ''}${md5(namespace)}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here
const checksum = key ?? md5(`${name}${jobId ?? ''}${md5(namespace)}`);
This feature is really needed for me and this pr is open for quite long. Is it still planned to support this feature? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
ref #1822