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

feat: Add getCronFromFrequency to manifest models #1325

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

doubleface
Copy link
Contributor

This feature is imported from harvest and allows to get the cron string
from a given frequency like 'daily'.

This cron string will configure the trigger associated to the konnector

.vscode/settings.json Outdated Show resolved Hide resolved
@doubleface doubleface force-pushed the feat/cron branch 5 times, most recently from 9121f3d to 0d8e7ac Compare March 6, 2023 13:50
Copy link
Contributor

@paultranvan paultranvan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can now create daily, weekly and monthly triggers, with the randomness directly handled by the stack: https://github.com/cozy/cozy-stack/blob/master/docs/jobs.md#daily-syntax
I assume we could eventually directly use this? Although hourly is not supported yet (but should it?)

packages/cozy-client/src/models/manifest.js Outdated Show resolved Hide resolved
*
* @param {import('../types').IOCozyKonnector} konnector - io.cozy.konnectors object
* @param {Date} startDate - start date
* @param {function} [randomDayTimeFn] - function generating random hours and minutes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth mentioning that the function requires 2 params, the minimal start hour and the max end hour, in the [0-24] interval? Which, I think, is a bit weird as it restricts the possibility to pass a custom function, but I don't know the use-case for this

This feature is imported from harvest and allows to get the cron string
from a given frequency like 'daily'.

This cron string will configure the trigger associated to the konnector
@doubleface
Copy link
Contributor Author

You can now create daily, weekly and monthly triggers, with the randomness directly handled by the stack: cozy/cozy-stack@master/docs/jobs.md#daily-syntax

Oh I did not know that. Let's use it then

@doubleface
Copy link
Contributor Author

Although hourly is not supported yet (but should it?) @paultranvan

I just saw that some some banking konnectors used by self hosted users use the "hourly" frequency (because the konnector is only run for them is their own machine I suppose). Maybe we should add the hourly frequency to the stack, what do you think ?

@doubleface
Copy link
Contributor Author

doubleface commented Mar 8, 2023

@paultranvan It looks like hourly frequency is actually accepted but not documented

But actually no, hourly is only implemented when the stack creates the trigger itself (when receiving a BI webhook for a non existing account/trigger)

Adding @hourly trigger support should not be a lot work then

@Crash--
Copy link
Contributor

Crash-- commented Mar 10, 2023

Let's see cozy/cozy-stack#3835

@Crash--
Copy link
Contributor

Crash-- commented Mar 13, 2023

@doubleface @hourly has been added to the stack. But I think we should use it with a lot of precaution. For the two examples you have listed, I think we should move them @daily.

@Crash--
Copy link
Contributor

Crash-- commented Mar 20, 2023

ping @doubleface

@Crash--
Copy link
Contributor

Crash-- commented May 9, 2023

@doubleface 🙏

@Crash--
Copy link
Contributor

Crash-- commented Jun 22, 2023

@doubleface

@Crash--
Copy link
Contributor

Crash-- commented Feb 5, 2024

@doubleface . If not needed anymore, please close.

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.

3 participants