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

Add Routine support #235

Closed
Zabuzard opened this issue Oct 30, 2021 · 7 comments · Fixed by #346
Closed

Add Routine support #235

Zabuzard opened this issue Oct 30, 2021 · 7 comments · Fixed by #346
Assignees
Labels
enhancement New feature or request priority: major

Comments

@Zabuzard
Copy link
Member

Zabuzard commented Oct 30, 2021

More and more commands need to execute code on a scheduled routine (for example every x minutes or once per day or once at a specific date).

The command system should offer a generic way to all commands (also, not just SlashCommands) to schedule their actions on a central managed scheduler.

The interface should be easy and convenient to use for all registered commands. It should at least offer ways to:

  • execute code at date/time (send birthday greetings to zabu at 8th november)
  • execute code after a specific waiting time (expire this message in 2 days)
  • repeatedly execute code (check the audit logs every 5 hours)
  • optionally persist a schedule, so that it is not lost if the bot is shutdown (/remind me at ... command)

The exact needs are not yet known but commands wanting to use such mechanisms are slowly comming (mod audit log, top helper, free command, ...)


Edit: Removed any sort of persisted-one-shot-tasks from the scope of this issue (see comments for why).

@Zabuzard Zabuzard added the enhancement New feature or request label Oct 30, 2021
@Zabuzard Zabuzard added this to the Improvement phase 1 milestone Oct 30, 2021
Zabuzard added a commit that referenced this issue Dec 10, 2021
this change is not super nice design but it is necessary for now, since routines and listeners need access to systems such as `TagSystem`, `ModAuditLogStore` and similar, which are created there.

the plan is to make all of this nicer with #235 and #236
Zabuzard added a commit that referenced this issue Dec 11, 2021
this change is not super nice design but it is necessary for now, since routines and listeners need access to systems such as `TagSystem`, `ModAuditLogStore` and similar, which are created there.

the plan is to make all of this nicer with #235 and #236
Zabuzard added a commit that referenced this issue Dec 14, 2021
this change is not super nice design but it is necessary for now, since routines and listeners need access to systems such as `TagSystem`, `ModAuditLogStore` and similar, which are created there.

the plan is to make all of this nicer with #235 and #236
JJeeff248 pushed a commit to JJeeff248/TJ-Bot that referenced this issue Dec 17, 2021
this change is not super nice design but it is necessary for now, since routines and listeners need access to systems such as `TagSystem`, `ModAuditLogStore` and similar, which are created there.

the plan is to make all of this nicer with Together-Java#235 and Together-Java#236
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@github-actions github-actions bot added the stale label Dec 17, 2021
@Zabuzard
Copy link
Member Author

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

This is actually still relevant, we need it eventually.

@Zabuzard Zabuzard removed the stale label Dec 17, 2021
@github-actions
Copy link

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

@Zabuzard
Copy link
Member Author

I feel like this has to be split into two separate topics:

  1. support for executing something on a schedule (the actual routine), e.g. every 5 mins (also, doesnt need persistance)
  2. add a service that is accessible to all features, allowing them to schedule one-shot tasks at a given date/time or with a delay, which is persisted in case the bot shuts down.

@Zabuzard
Copy link
Member Author

Regarding the second part, we decided that there is no pleasant way to offer a generic scheduler solution for persisted-one-shot-tasks.

The problem is that its pretty much impossible to persist a generic Runnable (the task to persist) in the database in a non-fragile way. One possibility was to persist something like

@FunctionalInterface
public interface Task extends Serializable {
  void run(JDA jda);
}

(with Java serialization or JSON or similar)
which at least has the benefit that it can inject a fresh JDA, while the user can choose what exactly to serialize and what not. However, it is still too fragile for practical use. For example, suppose a /remind-me what:"happy birthday" when:2 years, when this is executed in 2 years, chances are high that JDA (or another component in the code base) was updated in a way that makes the old serialized code invalid (for example, because the Discord end-point to post messages has changed).
Alternative approaches, such as simply storing the name of a static method to execute (so that only the location of the code but not the code itself is serialized), are not powerful enough and too limiting.

We concluded that users interested in a persisted-scheduled-one-shot-task features need to roll a custom domain-specific solution each.

However, we agreed that a big part of that can still be taken by a generic helper solution that assists users in creating such a custom system. It could for example register listeners by name and then persist simple content, such as the name of the interested listeners, together with the schedule. That way, it could do all the heavy lifting of the task scheduling but instead of triggering persisted tasks, it merely triggeres persisted listeners, who know better how to do the actual task at hand.

Before we create such a system however, we first want to wait for one or two users to create custom solutions. It is much easier to identify and extract the duplicated logic and put them into a helper system in hindsight.

@borgrel
Copy link
Contributor

borgrel commented Jan 19, 2022

Agreed, Runnables should NOT be persisted in the database however there is no problem with using the database to persist the routines

u dont need special serializable functionality or some stuff

public interface RoutineBooter {
  Collection<Routines> loadRoutines(BD db);
}

if u do this the routines can still be stored in the db (for example a table containing a list of channels can be stored in the database and converted into a Routine that monitors those channels) (or a table of users can be converted into a routine that mutes those users on rejoin)

tl:dr storing runnables in the database is DEFINTELY a bad idea, however there is absolutely nothing wrong with storing the data thats needed to generate runnables in a custom manner

@Zabuzard
Copy link
Member Author

Zabuzard commented Jan 19, 2022

Yeah. I didnt saw a need to persist routines ("execute me every 5 minutes") yet though. Their schedule can just be restarted upon bot launch. "Bot launches; Hey routine, whats ur schedule again? Okay will execute u each 5 mins."

The persistence bit is only interesting for one-shot tasks like /remind-me and similar. In which case the part that will be persisted should likely just be the "link" to the callsite. For example with by-name-registration (similar to how we persist the command routing). Right now we have no single user for such a feature yet though, I would like to wait for /remind-me or similar to be coded before adding a helper system for them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request priority: major
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants