-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
Event Unique Key implementation #80
base: 3.8
Are you sure you want to change the base?
Conversation
PR sent again as requested. |
I have reviewed implementation and it is quite good, but I'm wondering is it address issues in #33? Ids are still "not-stable", maybe it will better to allow users to set their own id? |
Personalized Id could be a good idea but i think it could also lead to errors or confusion. When you have several people entering tasks for the same company or group you may end up using the same custom ID for different tasks. At that point, by introducing a launcher per custom task ID, which one will you launch? At that point you would have to have a system that checks if there are tasks with the same custom ID and in my opinion it would become a mess. In our company we use crunz to manage automatic processes. There are currently more than 200 tasks configured. Handling uniqueness on the personalized ID would be a problem. But in the end you are the project leader... so I leave the choice up to you :) hihi.. |
Here is the thing, at "collecting tasks" part should be check that all users' ids (alnum BTW) are unique, if not throws exception. What do You think? Also, please note that |
Regarding the first point, the solution you propose could be valid. The only perplexity is that we can generate new tasks with the vendor/bin/crunz make:task function but also with a vi/nano or by moving them to the folder with an sftp. In the first case the exception could easily be inserted into the process of adding a new task. In other cases where would you put the exception? It cannot be placed in the engine that executes the tasks because you would also block tasks that are not affected by the problem and would not start. As regards the second point, when generating the Unique ID I tried to use as much information as possible to uniquely locate the file. As you can see I used the path of the task file inside the file folder, the description and the cron expression. In that case they would have two identical IDs and it would actually be a problem. However, we could add in the statement \md5($this->sourceFile . $this->description . $this->expression); also the event sequence within a tasks.php file. In that case it will certainly be unique: However, how do you prefer development to proceed? Unique identifier indicated or calculated? |
In defensive programming any problem should fail fast and loud so I don't see a problem in throwing exception when collecting tasks, Crunz can't proceed with duplicated task'id.
Indicated, and if not then generated. |
In the PR there is a possible solution to calculate a constant unique identifier for events.
This solution is linked to issue #33 and could be the first piece towards creating a launcher that allows you to launch a task based on the unique identifier.