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 spawn kill mix-up. #2409

Merged
merged 1 commit into from
Aug 25, 2017
Merged

Fix spawn kill mix-up. #2409

merged 1 commit into from
Aug 25, 2017

Conversation

hjoliver
Copy link
Member

The (infrequently used!) spawn command was killing target tasks ... looks like a cut-n-paste error in the new network layer.

As this is a bug, and easily fixed, we'd better put it in next release. Clearly we need cylc spawn tests with this - but I won't have time tonight.

@hjoliver hjoliver added this to the next release milestone Aug 24, 2017
@hjoliver hjoliver self-assigned this Aug 24, 2017
@hjoliver hjoliver added the bug Something is wrong :( label Aug 24, 2017
@matthewrmshin
Copy link
Contributor

Sorry I had actually spotted this while doing#2373, but had forgotten to raise a separate fix.

@matthewrmshin
Copy link
Contributor

Perhaps we should have a better test for the command?

@matthewrmshin
Copy link
Contributor

Can't argue with the change otherwise.

@matthewrmshin
Copy link
Contributor

(In this case, test should not be a blocker for the release.)

@hjoliver
Copy link
Member Author

perhaps we should have a better test for the command?

Yes we should - a quick grep suggests we don't have any for it.

@hjoliver
Copy link
Member Author

(In this case, test should not be a blocker for the release.)

@oliver-sanders has just raised a new bug (2411) and labeled it for next release as well - and it is sufficiently bad (log duplication) that I think we should wait on that first anyway ...

@hjoliver
Copy link
Member Author

(In this case, test should not be a blocker for the release.)

Yeah, I'll merge this - it's just a one liner obvious fix. Now we have @oliver-sanders new bug though (2411), which i think is sufficiently serious to wait on ...

@hjoliver hjoliver merged commit 848f840 into cylc:master Aug 25, 2017
@hjoliver hjoliver deleted the spawn-not-kill branch August 25, 2017 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :(
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants