Skip to content

refactor: remove submit_subtasks_from_json#1479

Merged
prioux merged 2 commits intoaces:masterfrom
Freedisch:remove_subtasks_json
Mar 5, 2025
Merged

refactor: remove submit_subtasks_from_json#1479
prioux merged 2 commits intoaces:masterfrom
Freedisch:remove_subtasks_json

Conversation

@Freedisch
Copy link
Copy Markdown
Contributor

Solved issue #1470

The following methods should be removed entirely from the ClusterTask class:

  • submit_subtasks_from_json
  • submit_task_from_string
  • validate_json_string

and submit_subtasks_from_json call in the bourreau_worker

Signed-off-by: freedisch <freeproduc@gmail.com>
Copy link
Copy Markdown
Member

@prioux prioux left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution. THere are just a few more minor adjustments to the code I'd like made, see my comments (basically, more lines to remove). Once this is done I'll merge the PR.

Comment thread Bourreau/app/models/bourreau_worker.rb Outdated
if task.class.properties[:can_submit_new_tasks].present?
task.submit_subtasks_from_json
end

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The comment block at lines 286-288 is an explanation for the lines you removed, so these comments should also be removed. Leave a single blank line before the final return please.

Comment thread BrainPortal/app/models/cluster_task.rb Outdated
@@ -2611,147 +2611,8 @@ def install_ext3fs_filesystem(filename,size) #:nodoc:

public
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The public keyword here affects all declarations the follows in the file. In this case, it affected the method you deleted. Just after that is the keyword protected which affects the other method you deleted. And after that follows code for "Singularity Load" which stays the same and has its own private keyword. Therefore, I think you should also remove the public and private which are now useless.

@prioux
Copy link
Copy Markdown
Member

prioux commented Mar 5, 2025

Note to myself: after merge, clean up a few remaining connections with the new integrator, e.g. the class 'properties' of the task in BourreauClusterTask. Leave the old integrator alone, it's deprecated.

Signed-off-by: freedisch <freeproduc@gmail.com>
@prioux
Copy link
Copy Markdown
Member

prioux commented Mar 5, 2025

Thanks. As soon as the tests finish I'll squash-merge.

@prioux prioux merged commit 79a1aa1 into aces:master Mar 5, 2025
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.

Remove the "submit internal subtasks from JSON" feature

2 participants