Scheduler API #89

graywolf336 · 2018-11-05T23:14:11Z

To Do list:

Review codebase and merge conflicts
Find a new cron parser (as the package later is no longer active)
Add tests covering the Scheduler components
Implement bridges on Rocket.Chat side

codecov · 2018-11-05T23:16:28Z

Codecov Report

❗ No coverage uploaded for pull request base (master@18d30f9). Click here to learn what that means.
The diff coverage is 57.14%.

@@            Coverage Diff            @@
##             master      #89   +/-   ##
=========================================
  Coverage          ?   54.94%           
=========================================
  Files             ?       70           
  Lines             ?     2448           
  Branches          ?      362           
=========================================
  Hits              ?     1345           
  Misses            ?     1103           
  Partials          ?        0

Impacted Files	Coverage Δ
src/server/accessors/ConfigurationExtend.ts	`100% <100%> (ø)`
src/server/managers/AppAccessorManager.ts	`100% <100%> (ø)`
src/server/accessors/Reader.ts	`100% <100%> (ø)`
src/server/accessors/SchedulerRead.ts	`22.22% <22.22%> (ø)`
src/server/accessors/Modify.ts	`90% <33.33%> (ø)`
src/server/accessors/SchedulerExtend.ts	`40% <40%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18d30f9...5235256. Read the comment docs.

mrsimpson

Looks quite nice to me, though very dependent on later. The interfaces and types look well designed. The biggest headache to me is i18n.

src/definition/accessors/IJobBuilder.ts

mrsimpson · 2018-11-06T14:46:57Z

src/definition/scheduler/Schedule.ts

+}
+
+export class Schedule implements ScheduleData {
+    public static fromText(text: string): Schedule {


ahh, now seeing you included the factory method here. Then, two questions:

Why did you integrated it into the actual Schedule (and not a separate factory, except laziness ;) )?

Why did you have this wrapped in the JobBuilder as well?

So people can easily generate a schedule outside of the builder.

Makes the builder an all-in-one, trying to make things easy for developers.

src/definition/scheduler/Schedule.ts

rodrigok · 2018-11-06T17:45:11Z

@graywolf336 is this preventing the jib to run multiple times when running rocket.chat in a cluster?

rodrigok · 2018-11-06T17:48:16Z

@graywolf336 do you think this needs to be defined only inside the configuration method? Would be nice to define cron jobs based on messages or slashcommands, I think this should be something in the app class context.

graywolf336 · 2018-11-06T18:24:55Z

@rodrigok Yes, it will only run the job once even when there are multiple instances of the App system running.

And it will be available to Apps all the time, not just at initialization time.

There is still a lot of work to do, hence the "in progress" label. The reason this pull request was created was to get feedback from people, such as what @mrsimpson provided. 🙂

dbrekelmans

Looking forward to this being implemented! Would be very useful.

package.json

dbrekelmans · 2019-06-08T21:09:49Z

src/definition/accessors/ISchedulerExtend.ts

+import { IJob } from '../scheduler';
+import { IJobBuilder } from './IJobBuilder';
+
+export interface ISchedulerExtend {


I think this interface is missing some features.

You want to be able to unschedule jobs and get a list of scheduled jobs

So, what I'm going to do is add a new accessor to to the modify stage to allow modification to the jobs. But if I get into it and that is not a fluid design or works in reality, then we will see how else we can work with this.

src/definition/scheduler/IJob.ts

dbrekelmans · 2019-06-11T11:14:54Z

@graywolf336 Is there anything I can do to help you work on this feature? I'm not very familiar with the apps-engine (only played around a bit with creating an app), but I'm willing to dedicate some time to help you work on this.

d-gubert · 2019-10-07T20:24:08Z

@dbrekelmans hey! nice to know you're interested in helping us with this!

We're bringing this back up in our queue and will be dedicating time to the feature in the following weeks, and would definitely appreciate the help! We've updated the description to reflect what is still missing to get this published :)

Also, we switched to only supporting crontab style scheduling as of right now. In the future we may support a form of i18n scheduling. This decision was reached after we decided we wanted this feature out now instead of another year from now (aka two years after the feature was first attempted).

graywolf336 · 2019-10-07T22:53:01Z

src/definition/scheduler/IJob.ts

+     * user friendly since the list of job names will be visible
+     * to the server administrator.
+     */
+    name: string;


Thoughts to everyone watching this thread about requiring an id: string property? As right now the name: string is simply for displaying on the UI the list of jobs and a history of them. But if an App wants to cancel a job or view the job's history there is not a way to do it currently. My idea is to allow the App to define the property but if the app leaves it empty then a random one will be generated.

Couldn't we use the name for that? Prepend it with the appId and a colon and should be good to go IMO 😛

For our internal usage, yes. But for Apps to query, no. Because what if they want the same name for a job that runs based upon something a user does but doesn't want that detail to show up in the job history and they want to query all of the jobs done by a user? That's why I'm considering an id..

Because what if they want the same name for a job that runs based upon something a user does but doesn't want that detail to show up in the job history and they want to query all of the jobs done by a user?

I'm not sure I quite understand what you mean by this. Could you maybe give a small example?

Let's say you only want one job to run but you have two internal jobs and their behavior is massively based upon a configuration. The setting is changed in the admin area and when that happens you want to replace the job, that way the admin isn't aware of the different types and only sees one job in the job history and that type of stuff. Does that make sense?

I don't understand why there would be one job running multiple internal jobs. Shouldn't that just be separate jobs then?

Imagine a payroll or something. For auditing purposes there should only appear as one job in the log. But because payroll is an extremely complicated item, the developer wants to separate the logic out from one giant job into different ones. So internally there are several jobs by the same name that way it appears as one job. I know this is an example that is stretching it, but I do see some valid reasons for not having the name be unique.

gotta admit I didn't follow the use case either :/

I think I understand what @graywolf336 means.
IMO if an app wants to split one scheduled job into multiple parts, then that's something the app itself can solve and not something that should be solved by the scheduling API

And that's why I'm think we probably should have an id for the jobs and not rely on the name 🤔

graywolf336 · 2019-10-07T22:53:58Z

src/definition/scheduler/Schedule.ts

@@ -0,0 +1,35 @@
+import { parseExpression } from 'cron-parser';


https://www.npmjs.com/package/cron-parser

dbrekelmans · 2019-10-08T17:16:31Z

Hey @d-gubert, I'm still interested in helping, but I'd need something concrete to work on. Maybe we can find something tomorrow. I just joined open.rocket.chat, so we could chat there if you'd like.

I'm definitely going to write a remind-me app once this API is available. If it's up to standards maybe it could serve as an example for people looking to use this API.

graywolf336 · 2019-10-08T18:26:13Z

@dbrekelmans what's your username on the oepn.rocket.chat server?

dbrekelmans · 2019-10-08T18:41:33Z

@graywolf336 it's dbrekelmans, same as my github name.

sveatlo · 2020-03-27T13:27:52Z

Any progress on this? I need it desperately. The lack of a scheduler API is the last thing blocking me from completing a daily standup application for Rocket.Chat (like standup-raven for Mattermost), which in turn is the last bit I need to migrate the entire company to Rocket.Chat.
I am also willing to help with this merge request, but I'd need quite specific instructions about what to do, since my codebase knowledge is pretty much none at this point (I only tried creating like 2 rather simple applications). Let me know if I could help you.

graywolf336 · 2020-03-27T19:09:39Z

@sveatlo can you try out the current code base and get a feel for deploying an App using the current API? Then provide us feedback on it and ways we can improve?

majklovec · 2020-06-24T08:03:09Z

Any progress?

cuonghuunguyen · 2020-08-17T05:00:41Z

I am looking forward to it :D

graywolf336 · 2021-11-30T22:26:30Z

Closing this out dated PR in favor of the new functionality that has been in the Apps Engine for a while now. :)

Initial design of the scheduler api design

ca724e8

graywolf336 added the ⏳ in progress Items which are currently a work in progress label Nov 5, 2018

mrsimpson reviewed Nov 6, 2018

View reviewed changes

dbrekelmans reviewed Jun 8, 2019

View reviewed changes

graywolf336 mentioned this pull request Oct 3, 2019

Split out the modifiers into their own files #151

Merged

Merge master into scheduler

9915fd3

d-gubert mentioned this pull request Oct 7, 2019

Support for other languages darkeyedevelopers/natural-cron.js#4

Closed

graywolf336 commented Oct 7, 2019

View reviewed changes

Some a few bits for the scheduler

75382a3

shiqimei mentioned this pull request Nov 22, 2019

[NEW] Apps-Engine event for when a livechat room is closed RocketChat/Rocket.Chat#15837

Merged

graywolf336 added 3 commits January 23, 2020 15:39

Merge master into scheduler branch

8008231

WHoopies 🙈 forgot to ensure it compiled

8641cc7

Various clean up of items

5235256

thassiov mentioned this pull request May 14, 2020

Implement Scheduler #26

Open

thassiov linked an issue May 14, 2020 that may be closed by this pull request

Implement Scheduler #26

Open

graywolf336 mentioned this pull request Jul 4, 2020

[Question] Is there a way to use setTimeout() or setInterval() in Rocket.Chat App? #278

Closed

murtaza98 mentioned this pull request Jul 16, 2020

[FEATURE] Option to keep dialogflow session alive while user is in a handed-off session RocketChat/Apps.Dialogflow#30

Open

graywolf336 closed this Nov 30, 2021

graywolf336 deleted the scheduler branch November 30, 2021 22:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scheduler API #89

Scheduler API #89

graywolf336 commented Nov 5, 2018 •

edited

codecov bot commented Nov 5, 2018 •

edited

mrsimpson left a comment •

edited

mrsimpson Nov 6, 2018

graywolf336 Nov 6, 2018

rodrigok commented Nov 6, 2018

rodrigok commented Nov 6, 2018

graywolf336 commented Nov 6, 2018

dbrekelmans left a comment

dbrekelmans Jun 8, 2019

graywolf336 Oct 7, 2019

dbrekelmans commented Jun 11, 2019

d-gubert commented Oct 7, 2019

graywolf336 Oct 7, 2019 •

edited

d-gubert Oct 8, 2019

graywolf336 Oct 8, 2019

dbrekelmans Oct 8, 2019 •

edited

graywolf336 Oct 8, 2019

dbrekelmans Oct 8, 2019

graywolf336 Oct 8, 2019

d-gubert Oct 8, 2019

dbrekelmans Oct 8, 2019

graywolf336 Oct 8, 2019

graywolf336 Oct 7, 2019

dbrekelmans commented Oct 8, 2019 •

edited

graywolf336 commented Oct 8, 2019

dbrekelmans commented Oct 8, 2019

sveatlo commented Mar 27, 2020

graywolf336 commented Mar 27, 2020

majklovec commented Jun 24, 2020

cuonghuunguyen commented Aug 17, 2020

graywolf336 commented Nov 30, 2021

		@@ -0,0 +1,35 @@
		import { parseExpression } from 'cron-parser';

Scheduler API #89

Scheduler API #89

Conversation

graywolf336 commented Nov 5, 2018 • edited

codecov bot commented Nov 5, 2018 • edited

Codecov Report

mrsimpson left a comment • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rodrigok commented Nov 6, 2018

rodrigok commented Nov 6, 2018

graywolf336 commented Nov 6, 2018

dbrekelmans left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbrekelmans commented Jun 11, 2019

d-gubert commented Oct 7, 2019

graywolf336 Oct 7, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbrekelmans Oct 8, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbrekelmans commented Oct 8, 2019 • edited

graywolf336 commented Oct 8, 2019

dbrekelmans commented Oct 8, 2019

sveatlo commented Mar 27, 2020

graywolf336 commented Mar 27, 2020

majklovec commented Jun 24, 2020

cuonghuunguyen commented Aug 17, 2020

graywolf336 commented Nov 30, 2021

graywolf336 commented Nov 5, 2018 •

edited

codecov bot commented Nov 5, 2018 •

edited

mrsimpson left a comment •

edited

graywolf336 Oct 7, 2019 •

edited

dbrekelmans Oct 8, 2019 •

edited

dbrekelmans commented Oct 8, 2019 •

edited