juju / python-libjuju

Python library for the Juju API
Apache License 2.0
57 stars 93 forks source link

fix: machine scp & ssh #1020

Closed yanksyoon closed 4 months ago

yanksyoon commented 5 months ago

Description

This is a follow-up PR to #1016 , which contains cherry-picked commits without type imports.

Fİxes #997

QA Steps

\<Commands / tests / steps to run to verify that the change works:>

tox -e py3 -- tests/unit/...
tox -e integration -- tests/integration/test_machine.py

All CI tests need to pass.

\<Please note that most likely an additional test will be required by the reviewers for any change that's not a one liner to land.>

Notes & Discussion

\<Additional notes for the reviewers if needed. Please delete section if not applicable.>

jujubot commented 5 months ago

Thanks for opening a pull request! Please follow the instructions here to ensure your pull request is ready for review. Then, a maintainer will review your patch.

@hpidcock @anvial

jujubot commented 5 months ago

Thanks for opening a pull request! Please follow the instructions here to ensure your pull request is ready for review. Then, a maintainer will review your patch.

@hpidcock @anvial

cderici commented 4 months ago

Hi @yanksyoon, just pinging for this to let you know we're planning on making a release soon, so I'd love to roll this in if it's ready as well 👍

yanksyoon commented 4 months ago

@cderici Sorry! I was on my break, i'll get on this asap!

cderici commented 4 months ago

@cderici Sorry! I was on my break, i'll get on this asap!

@yanksyoon np, hope you had a good break!

Feel free to ping me whenever this is ready for review again 👍

yanksyoon commented 4 months ago

@cderici The tests seem to have failed due to changes in how juju handles old charm-store (cs:) urls. Looking into it!

yanksyoon commented 4 months ago

@cderici Changed! Thanks for the review! :D

cderici commented 4 months ago

@yanksyoon looks like the test_machine_ssh was failing in the CI, indicating a race that happened between the time the machine gets the IP and the ssh service to be up and running, so I just added a retry to get around that. I'll approve and land this after the tests run 👍 thanks again for working on this!

cderici commented 4 months ago

/build

cderici commented 4 months ago

/build

cderici commented 4 months ago

/build

cderici commented 4 months ago

/merge