Skip to content

Add SSH support for units and machines#393

Merged
jujubot merged 2 commits intojuju:masterfrom
ChrisMacNaughton:feature/ssh
Mar 10, 2020
Merged

Add SSH support for units and machines#393
jujubot merged 2 commits intojuju:masterfrom
ChrisMacNaughton:feature/ssh

Conversation

@ChrisMacNaughton
Copy link
Copy Markdown
Contributor

@ChrisMacNaughton ChrisMacNaughton commented Mar 4, 2020 •
edited by johnsca
Loading

Fixes #79

SimonRichardson
SimonRichardson requested changes Mar 4, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@SimonRichardson SimonRichardson left a comment

Choose a reason for hiding this comment

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

Comment just needs fixing, once that's done, happy to land.

Comment thread tests/integration/test_unit.py Outdated
@johnsca
Copy link
Copy Markdown
Contributor

johnsca commented Mar 4, 2020

provisioner.py already contains some SSH logic using the Paramiko library. I'm not entirely sure why that library was used instead of using an async subprocess like this PR does. I wonder if that dependency could be removed by refactoring that code to use this approach?

SimonRichardson
SimonRichardson approved these changes Mar 5, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@SimonRichardson SimonRichardson 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 the changes, can you raise a bug around updating scp to shell out.

@johnsca johnsca mentioned this pull request Mar 6, 2020
Closed
@ChrisMacNaughton
Copy link
Copy Markdown
Contributor Author

ChrisMacNaughton commented Mar 9, 2020

@SimonRichardson scp already does shell out?

@SimonRichardson
Copy link
Copy Markdown
Member

SimonRichardson commented Mar 9, 2020

$$merge$$

@SimonRichardson
Copy link
Copy Markdown
Member

SimonRichardson commented Mar 10, 2020

@ChrisMacNaughton can you rebase to the latest master, our merge bot won't let this land.

@SimonRichardson
Copy link
Copy Markdown
Member

SimonRichardson commented Mar 10, 2020

$$merge$$

@jujubot jujubot merged commit af10ed0 into juju:master Mar 10, 2020
@ChrisMacNaughton ChrisMacNaughton deleted the feature/ssh branch March 10, 2020 12:09
@hpidcock
Copy link
Copy Markdown
Member

hpidcock commented May 15, 2020

#422 fixes unit ssh async

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SimonRichardson SimonRichardson SimonRichardson approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Implement ssh

5 participants

@ChrisMacNaughton @johnsca @SimonRichardson @hpidcock @jujubot