Skip to content

remote: move non-file transfer methods from Remote#1626

Merged
batrick merged 1 commit intoceph:masterfrom
rishabh-d-dave:orch-remote-2
Apr 29, 2021
Merged

remote: move non-file transfer methods from Remote#1626
batrick merged 1 commit intoceph:masterfrom
rishabh-d-dave:orch-remote-2

Conversation

@rishabh-d-dave
Copy link
Contributor

@rishabh-d-dave rishabh-d-dave commented Mar 9, 2021 •
edited
Loading

Move methods that operate on a remote machine without depending on
another remote machine from orchestra.remote.Remote to a different class
in same module. This enables vstart_runner to reuse these methods.

Related PR: ceph/ceph#37667

@rishabh-d-dave rishabh-d-dave requested a review from batrick March 9, 2021 05:05
kshtsk
kshtsk reviewed Mar 10, 2021
View reviewed changes
kshtsk
kshtsk reviewed Mar 10, 2021
View reviewed changes
@kshtsk
Copy link
Contributor

kshtsk commented Mar 10, 2021

Could you please elaborate why this is required, and how this enable to 3rd party software to reuse something from teuthology?

batrick
batrick reviewed Mar 11, 2021
View reviewed changes
Copy link
Member

@batrick batrick left a comment

Choose a reason for hiding this comment

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

(Thought I finished this review yesterday but github must have left it incomplete.)

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Mar 11, 2021

@kshtsk

Could you please elaborate why this is required, and how this enable to 3rd party software to reuse something from teuthology?

Do you mean elaborate in the commit message?

@rishabh-d-dave rishabh-d-dave force-pushed the orch-remote-2 branch 9 times, most recently from 7982499 to fbadec9 Compare March 15, 2021 10:04
kshtsk
kshtsk reviewed Mar 15, 2021
View reviewed changes
kshtsk
kshtsk reviewed Mar 15, 2021
View reviewed changes
kshtsk
kshtsk reviewed Mar 15, 2021
View reviewed changes
@kshtsk
Copy link
Contributor

kshtsk commented Mar 15, 2021

Overall patch fbadec9 orchestra: fix flake8 issues in remote.py and remote_ops.py … doesn't do anything useful, the actual flake8 failures.

teuthology/orchestra/remote.py:16:1: F401 'teuthology.provision' imported but unused
teuthology/orchestra/remote.py:61:37: F821 undefined name 'get_reimage_types'

@rishabh-d-dave rishabh-d-dave mentioned this pull request Mar 15, 2021
Closed
@rishabh-d-dave rishabh-d-dave requested a review from kshtsk March 15, 2021 10:59
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Mar 15, 2021

@kshtsk I moved those commits to #1629 before your finished reviewing (I suppose) since those weren't related to this PR.

@rishabh-d-dave rishabh-d-dave force-pushed the orch-remote-2 branch 3 times, most recently from 372d8e4 to 23b963f Compare March 15, 2021 12:51
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Mar 17, 2021

@kshtsk @batrick PTAL.

batrick
batrick requested changes Mar 17, 2021
View reviewed changes
@rishabh-d-dave rishabh-d-dave force-pushed the orch-remote-2 branch 2 times, most recently from f734a3f to 4551056 Compare March 23, 2021 05:13
@rishabh-d-dave rishabh-d-dave requested a review from batrick March 23, 2021 05:42
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Mar 24, 2021

@batrick @kshtsk PTAL.

@rishabh-d-dave rishabh-d-dave force-pushed the orch-remote-2 branch 4 times, most recently from a9621bd to 30c16a2 Compare March 26, 2021 03:32
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Mar 30, 2021

@kshtsk PTAL.

@batrick
Copy link
Member

batrick commented Mar 31, 2021

needs rebase, changes look fine (but the diff is still weird, who knows why)

@rishabh-d-dave
orchestra: move methods for shell commands from remote.Remote
11c30bf 
Move methods that issue commands via shell and that don't necessarily
need to depend on SHH from class Remote to a different class. This
enables applications like vstart_runner.py (in Ceph repo) to reuse these
methods for running tests locally without necessarily depending on SSH
and without duplicating them in vstart_runner.py.

Signed-off-by: Rishabh Dave <ridave@redhat.com>
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Apr 1, 2021

@batrick I've rebased the PR. I am not exactly sure what causes the issue with the diff but it has something to do with moving of os() and arch(); see https://github.com/rishabh-d-dave/teuthology/commits/orch-remote-2-2.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Apr 1, 2021

@kshtsk PTAL.

@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Apr 21, 2021

@batrick @kshtsk Please take a look.

@batrick
Copy link
Member

batrick commented Apr 29, 2021

@susebot run deploy

@susebot
Copy link

susebot commented Apr 29, 2021

Commit 11c30bf is OK.
Check tests results in the Jenkins job: https://ceph-ci.suse.de/job/pr-teuthology-deploy/337/

@batrick batrick merged commit 81b2447 into ceph:master Apr 29, 2021
@rishabh-d-dave rishabh-d-dave deleted the orch-remote-2 branch April 30, 2021 09:58
@rishabh-d-dave
Copy link
Contributor Author

rishabh-d-dave commented Apr 30, 2021

@batrick Thank you for the review and for merging the PR.

@rishabh-d-dave rishabh-d-dave mentioned this pull request Apr 30, 2021
Merged
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@batrick batrick batrick approved these changes

@kshtsk kshtsk Awaiting requested review from kshtsk

Assignees

No one assigned

Labels

improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@rishabh-d-dave @kshtsk @batrick @susebot