feature/#1197 pass job as metadata for event #1277
Conversation
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
There was a problem hiding this comment.
- We are missing the unit tests covering the new code on both reload.py and submission.py modules.
- Even if the code looks smart, I don't like giving the notification responsibility to the entity itself. For me, it should be the repository/self_setter layer.
We can discuss and try to challenge your solution to see if we can propose a better design.
| }[ | ||
| manager | ||
| ] # type: ignore | ||
| }[manager] # type: ignore |
There was a problem hiding this comment.
WHy do we need to add a "# type: ignore" here?
There was a problem hiding this comment.
apparently we don't need it!
jrobinAV
added
Core
toan-quach
force-pushed
the
feature/#1197-add-job-to-on-submission-change-event
branch
from
44e9311
to
a47bac4
Compare
| EventOperation.UPDATE, | ||
| "submission_status", | ||
| submission._submission_status, | ||
| job_triggered_submission_status_changed=job.id, |
There was a problem hiding this comment.
This parameter name is exposed to the user through the event. It becomes an attribute of the event. We must find a better name. What about: "job_id"?
There was a problem hiding this comment.
a bit vague imho, how about trigger_job_id?
There was a problem hiding this comment.
it is a submission status change. I can't see any possible confusion using job_id instead of trigger_job_id. Plus, it is a technical consideration for me to add a prefix like trigger.
The event expresses a submission_status change for a submission entity due to a job update. I don't see any possible confusion.
What do you think? and @FlorianJacta?
toan-quach
force-pushed
the
feature/#1197-add-job-to-on-submission-change-event
branch
from
95b902f
to
73a2435
Compare
toan-quach
force-pushed
the
feature/#1197-add-job-to-on-submission-change-event
branch
from
2add572
to
7d827c5
Compare
toan-quach
requested review from
jrobinAV,
FredLL-Avaiga,
trgiangdo and
joaoandre-avaiga
toan-quach
deleted the
feature/#1197-add-job-to-on-submission-change-event
branch
#1197