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

Fix potential type error & a few small mistakes #6357

Merged
merged 4 commits into from
Sep 24, 2024

Conversation

MetRonnie
Copy link
Member

Small change with no associated issue

Check List

  • I have read CONTRIBUTING.md and added my name as a Code Contributor.
  • Contains logically grouped changes (else tidy your branch by rebase).
  • Does not contain off-topic changes (use other PRs for other changes).
  • No dependency changes
  • tests are not needed
  • No changelog entry as not known bug
  • No docs needed
  • If this is a bug fix, PR should be raised against the relevant ?.?.x branch.

@MetRonnie MetRonnie added bug? Not sure if this is a bug or not small labels Sep 4, 2024
@MetRonnie MetRonnie added this to the 8.3.4 milestone Sep 4, 2024
@MetRonnie MetRonnie self-assigned this Sep 4, 2024
cylc/flow/task_pool.py Outdated Show resolved Hide resolved
@MetRonnie MetRonnie added question Flag this as a question for the next Cylc project meeting. and removed question Flag this as a question for the next Cylc project meeting. labels Sep 4, 2024
@MetRonnie
Copy link
Member Author

MetRonnie commented Sep 5, 2024

Another similar mistake spotted for the TaskPool.spawn_task() method (not to be confused with TaskEventsManager.spawn_func() mentioned before):

def spawn_task(
self,
name: str,
point: 'PointBase',
flow_nums: Set[int],
force: bool = False,
flow_wait: bool = False,
) -> Optional[TaskProxy]:

flow_wait is incorrectly passed to the force argument in 2 places:

# ntask does not exist: spawn it in the flow.
ntask = self.spawn_task(tdef.name, point, flow_nums, flow_wait)

def _set_prereqs_tdef(
self, point, taskdef, prereqs, flow_nums, flow_wait
):
"""Spawn a future task and set prerequisites on it."""
itask = self.spawn_task(taskdef.name, point, flow_nums, flow_wait)

@MetRonnie MetRonnie added the question Flag this as a question for the next Cylc project meeting. label Sep 5, 2024
@hjoliver
Copy link
Member

hjoliver commented Sep 5, 2024

(Ah, yeah I found that in my group-trigger branch, which I haven't exposed to you all yet)

@MetRonnie MetRonnie changed the title Fix potential type error Fix potential type error & a few small mistakes Sep 6, 2024
@MetRonnie MetRonnie removed the question Flag this as a question for the next Cylc project meeting. label Sep 6, 2024
Copy link
Member

@hjoliver hjoliver left a comment

Choose a reason for hiding this comment

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

LGTM, just one question.

cylc/flow/data_store_mgr.py Show resolved Hide resolved
@MetRonnie

This comment was marked as resolved.

@oliver-sanders oliver-sanders modified the milestones: 8.3.4, 8.3.5 Sep 12, 2024
@oliver-sanders oliver-sanders merged commit 8110813 into cylc:8.3.x Sep 24, 2024
26 of 27 checks passed
@MetRonnie MetRonnie deleted the type-safety branch September 24, 2024 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug? Not sure if this is a bug or not small
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants