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

PM job state management #3702

Merged
merged 3 commits into from
Aug 1, 2024
Merged

PM job state management #3702

merged 3 commits into from
Aug 1, 2024

Conversation

ashton22305
Copy link
Contributor

Fixes #3685

Updates the job log whenever a job is queried from the adapter, and does not query when the job is logged as being completed.

@@ -103,12 +103,13 @@ def destroy

# GET /projects/:project_id/jobs/:cluster/:jobid
def job_details
project = Project.find(params[:project_id])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should avoid using params directly and instead us maybe job_details_params so the list of parameters we're accessing are known to be valid and permitted.

Comment on lines +206 to +212
def job_from_id(job_id, cluster)
launchers = Launcher.all(directory)
launchers.each do |launcher|
job = launcher.job_from_id(job_id, cluster)
return job unless job.nil?
end
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's something wrong here that I think I started. jobs are associated with the launchers and that's why you have to do all this weirdness here. But I think a better/simpler paradigm really is that the project itself keeps the job log and the entries have a relationship to the launcher (if we even need such a relationship).

Which is really just to say that I think we should rethink & refactor this whole paradigm before it gets worse. I think we can move forward with this as it is, but will likely have to change it almost immediately.

@johrstrom johrstrom merged commit e8272f6 into master Aug 1, 2024
26 checks passed
@johrstrom johrstrom deleted the job-state-management branch August 1, 2024 20:11
ashton22305 added a commit that referenced this pull request Aug 2, 2024
* better pm job state management

* move job search to function

* use job_details_params
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

better job state management in pm
3 participants