Refactor endpoints post Runtime integration #1733
Conversation
ElePT
force-pushed
the
refactor-runtime-endpoints
branch
from
e1c927f to
a1cac25
Compare
…Add use cases for associating and retrieving runtime jobs. Adapt tests to new messages.
ElePT
force-pushed
the
refactor-runtime-endpoints
branch
from
c6363cf to
acf8201
Compare
korgan00
left a comment
korgan00
left a comment
There was a problem hiding this comment.
Code wise it looks good to me.
Should we control something about permissions in the Post?
|
@ElePT I think you said this was ready for review, right? Can I click on "Ready of review"? |
Co-authored-by: Goyo <korgan00@gmail.com>
| RuntimeJob.objects.create( | ||
| job=job, | ||
| runtime_job=runtime_job, | ||
| runtime_session=runtime_session, | ||
| ) |
There was a problem hiding this comment.
I know accessing to the Django orm in use cases is tempting, but if we are following the Repository approach, all the database accesses (that means, any call to Model.objects.whatever()) should be done through a repository.
There was a problem hiding this comment.
In this specific case, this use case is very simple. I guess you can get rid of the use case and call to the repository directly from the endpoint. Having one single call to one repository in the controller is acceptable.
There was a problem hiding this comment.
This is a good point, this case is so simple that I struggled to find the balance between following the approach of other endpoints and adding too much boilerplate. I am fine updating it.
| class GetRuntimeJobsUseCase: | ||
| """Retrieve all RuntimeJob objects associated to a given Job.""" | ||
|
|
||
| jobs_repository = JobsRepository() | ||
|
|
||
| def execute(self, job_id: UUID) -> list[RuntimeJob]: | ||
| """ | ||
| Return all RuntimeJob objects associated to a given Job. | ||
| Args: | ||
| job_id: Target Job ID. | ||
| Returns: | ||
| The list of RuntimeJobs. | ||
| Raises: | ||
| NotFoundError: If the job does not exist or access is denied. | ||
| """ | ||
| job = self.jobs_repository.get_job_by_id(job_id) | ||
|
|
||
| if job is None: | ||
| raise NotFoundError(f"Job [{job_id}] not found") | ||
|
|
||
| return job.runtime_jobs.all() |
There was a problem hiding this comment.
Same, just create (or reuse) a repository RuntimeRepository with this logic and call it from the endpoint. In the repository, you can safely call to any Model.objects.whatever() method you want :)
| @@ -1,8 +1,9 @@ | |||
| """ | |||
| Django Rest framework Job views for api application: | |||
There was a problem hiding this comment.
Initially now that we removed the end-points from this file we should be able to remove it entirely 👍
There was a problem hiding this comment.
ok, I will give it a try
4 participants
Summary
Details and comments
Follow-up of #1707