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

Confusing documentation for Job.set_rqmt and Job.update_rqmt #218

Open
jairsan opened this issue Nov 12, 2024 · 0 comments · May be fixed by #219
Open

Confusing documentation for Job.set_rqmt and Job.update_rqmt #218

jairsan opened this issue Nov 12, 2024 · 0 comments · May be fixed by #219

Comments

@jairsan
Copy link

jairsan commented Nov 12, 2024

The documentation of update_rqmt says "Updates the given requirements for this job, values not set in rqmt will not be affected.". So I had assumed that every time I called this function, it would update the rqmts of the task with the values of the new rqmt. However, internally this is not the case. The provided rqmt is stored and used later. Therefore, multiple calls overwrite the previous results.

See here:

    def update_rqmt(self, task_name, rqmt):
        """Updates the given requirements for this job, values not set in rqmt will not be affected.

        :param str task_name: Which task will be affected
        :param rqmt: the new requirements
        :return:
        """
        self._sis_task_rqmt_overwrite[task_name] = rqmt.copy(), False
        return self

This is then used later in the _sis_tasks method

                if task_name in self._sis_task_rqmt_overwrite:
                    rqmt, replace = self._sis_task_rqmt_overwrite[task_name]
                    if replace:
                        task._rqmt = rqmt
                    else:
                        task._rqmt.update(rqmt)

I was not the only one confused by this so I believe we should clarify the documentation.

See #219 for a proposed fix.

@jairsan jairsan linked a pull request Nov 12, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant