This repository has been archived by the owner on Apr 7, 2022. It is now read-only.
[WIP] Tests for grouping cluster candu graphs by tags #8677
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nachandr
force-pushed
the
candu_group_cluster_graph_by_tag
branch
from
976a23c to
e625631
Compare
nachandr
changed the title
nachandr
changed the title
dajoRH
changed the title
nachandr
force-pushed
the
candu_group_cluster_graph_by_tag
branch
from
e625631 to
4b5d59e
Compare
nachandr
changed the title
nachandr
changed the title
nachandr
force-pushed
the
candu_group_cluster_graph_by_tag
branch
from
4b5d59e to
96f7726
Compare
nachandr
changed the title
nachandr
force-pushed
the
candu_group_cluster_graph_by_tag
branch
from
e89f0e5 to
279db12
Compare
nachandr
changed the title
dajoRH
changed the title
nachandr
force-pushed
the
candu_group_cluster_graph_by_tag
branch
from
53e5402 to
9569ecc
Compare
nachandr
changed the title
nachandr
force-pushed
the
candu_group_cluster_graph_by_tag
branch
from
9569ecc to
2ba1dae
Compare
digitronik
suggested changes
Apr 15, 2019
There was a problem hiding this comment.
@nachandr check PRT failing also please glance on some comments.
Thanks for this PR 👍
| vm_host = vm.host.name | ||
| return temp_appliance_extended_db.collections.hosts.instantiate(name=vm_host) | ||
| elif entity == 'cluster': | ||
| provider = temp_appliance_extended_db.rest_api.collections.providers.get(id=vm.ems_id) |
There was a problem hiding this comment.
Required:
we have property for getting custer for specific vm
There was a problem hiding this comment.
@nachandr when we were looking at this the other day, we were using the entity object that was returned by the rest api. What @digitronik is suggesting here is to actually use the vm and cluster collections (this will use the GUI).
`In[17]: vm = appliance.collections.infra_vms.instantiate(name='cu-24x7', provider=vmwaretest)
In[18]: vm
Out[18]: InfraVm(name='cu-24x7', provider=VMwareProvider(endpoints={'default': <cfme.infrastructure.provider.virtualcenter.VirtualCenterEndpoint object at 0x7fdff3065650>}, name='vSphere 6.5 (nested)', key='vsphere65-nested', zone='default', start_ip='10.8.196.40', end_ip='10.8.196.40', provider_data=None), template_name=None)
In[19]: vm.cluster
Out[19]: Cluster(name=u'Cluster', provider=VMwareProvider(endpoints={'default': <cfme.infrastructure.provider.virtualcenter.VirtualCenterEndpoint object at 0x7fdff3065650>}, name='vSphere 6.5 (nested)', key='vsphere65-nested', zone='default', start_ip='10.8.196.40', end_ip='10.8.196.40', provider_data=None))
Nikhil and I did chat offline and this is slower with our current implementation. I think both ways work here.
| if entity == 'host': | ||
| view = navigate_to(entity_object, 'candu') | ||
| elif entity == 'cluster': | ||
| view = navigate_to(entity_object, 'Utilization') |
There was a problem hiding this comment.
aaaaaahhh this was my bad :). I think we need to use same name. TODO: need PR for this.
| data = {'interval': interval, 'group_by': gp_by} | ||
| view.options.fill(data) | ||
|
|
||
| entity_graph = entity + '_' + graph_type |
There was a problem hiding this comment.
optional:
I would use .format for string formatting
| # We have to use vm or host average graph for zoom in operation. | ||
| if entity == 'cluster': | ||
| graph_zoom = ["cluster_host", "cluster_vm"] | ||
| entity_graph = entity + '_' + graph_type |
| if entity == 'cluster': | ||
| graph_zoom = ["cluster_host", "cluster_vm"] | ||
| entity_graph = entity + '_' + graph_type | ||
| avg_graph = entity_graph if entity_graph in graph_zoom \ |
There was a problem hiding this comment.
Required:
please try to format this; try to avoid \
digitronik
changed the title
nachandr
changed the title
nachandr
force-pushed
the
candu_group_cluster_graph_by_tag
branch
from
7294a3b to
2a01cb2
Compare
|
I detected some fixture changes in commit 2a01cb2 The local fixture
Please, consider creating a PRT run against these tests make sure your fixture changes do not break existing usage 😃 |
dajoRH
changed the title
|
Would you mind rebasing this Pull Request against latest master, please? |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.