-
Notifications
You must be signed in to change notification settings - Fork 165
[1LP][RFR] Change from_provider to use get_mgmt instead of get_crud #9674
Conversation
451bf42
to
cc76c3f
Compare
cc76c3f
to
e42aa4b
Compare
Looks good, I shouldn't have allowed @john-dupuy to change it to an actual provider object. We didn't hit it until hooks were added to appliance stack push/pop. Below PR is for wrapping the hook execution and reducing default ssh wait time: Also discussing a separate change to where the hook methods are registered, to restrict them to being executed within a pytest context - when we don't have dummy appliance objects, and to reduce the load up time and actions when using appliance objects in local consoles. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sat next to you while we talked through the impact, so of course its just absolutely perfect.
|
||
# TODO Can we remove this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For vsphere this method is provided by the fixtures
integration_tests/cfme/fixtures/appliance.py
Lines 261 to 278 in 0fc21aa
@pytest.fixture(scope="function") | |
def configure_fleecing(appliance, provider, setup_provider): | |
vddk_url = get_vddk_url(provider) | |
provider.setup_hosts_credentials() | |
appliance.install_vddk(vddk_url=vddk_url) | |
yield | |
appliance.uninstall_vddk() | |
provider.remove_hosts_credentials() | |
@pytest.fixture(scope="module") | |
def configure_fleecing_modscope(appliance, provider, setup_provider_modscope): | |
vddk_url = get_vddk_url(provider) | |
provider.setup_hosts_credentials() | |
appliance.install_vddk(vddk_url=vddk_url) | |
yield | |
appliance.uninstall_vddk() | |
provider.remove_hosts_credentials() |
Unless we want to keep it as a convenience method for SSA testing. @sbulage do you make use of this method during
your SSA testing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@john-dupuy No I didn't write it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for pointing this out @john-dupuy, @jawatts and I discussed it and decided because of the larger assumptions and actions in configure_fleecing, we would leave it in place for now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me . well done !
No description provided.