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 problems with API-calls based on last_command_started #465

Closed

Conversation

maxmeyer
Copy link
Member

Summary

There are some API-calls which depend on `#last_command_started'. The behaviour changed with aruba >= 1.0.0. Now all commands need to be stopped explicitly. This PR clarifies this.

Motivation and Context

TODO:

  • Add examples for the new behaviour
  • Find places where we need to stop commands explicitly

How Has This Been Tested?

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Refactoring (cleanup of codebase withouth changing any existing functionality)
  • Update documentation

Checklist:

  • I've added tests for my code
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.

@maxmeyer
Copy link
Member Author

@mvz I think the changed code in the first commit causes the failing tests. What do would you expect as a user? Shall we discuss this in #432?

@mvz
Copy link
Contributor

mvz commented Jul 27, 2017

I wonder whether this is the same problem as #432, since that was with a pre-1.0 version.

@maxmeyer
Copy link
Member Author

maxmeyer commented Jul 27, 2017

Not sure. Will have a look into this in the next couple of day's, but feel free to help. Maybe it's a good idea to open a separate issue for discussion.

@mvz
Copy link
Contributor

mvz commented Jul 27, 2017

In terms of what I expect as a user, it is a bit tricky.

  • If I'm just running one command, I expect this step to wait for my command to finish.
  • If I run commands sequentially, I expect this step to wait for the command started last to finish (this is the Regression: exit status not waiting for run interactively process to complete #432 case I think).
  • If I'm running two commands in parallel, I probably expect this step to wait for the command I interacted with last to finish.

As a first approximation, I think the step the exit status should be <n> needs to wait for the last started command to exit. We can then see what special cases are needed for the third case above.

@junaruga junaruga mentioned this pull request Jul 28, 2017
8 tasks
@mvz
Copy link
Contributor

mvz commented Sep 5, 2017

I think this was fixed in #487.

@maxmeyer maxmeyer closed this Sep 5, 2017
@maxmeyer
Copy link
Member Author

maxmeyer commented Sep 5, 2017

Thanks @mvz for fixing this.

@maxmeyer maxmeyer deleted the feature/fix-api-based-on-last_command_stopped branch November 9, 2017 06:51
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.

2 participants