Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mark widgets as non timezone sensitive #14497

Merged
merged 1 commit into from
Apr 19, 2017

Conversation

kbrock
Copy link
Member

@kbrock kbrock commented Mar 24, 2017

Follow up to #14386 (to concept introduced in #14285 )

This change allows these widgets to run once per security group rather than once per timezone per security group. (expecting 1/2 the number of reports to run per widget)

Assumes: widgets that do not reference time-sensitive columns are not timezone-sensitive.

In the end, 12 reports do not need the timezone. 3 had already been updated, so this updates the remaining 9 report files used by widgets.


widget report need timezone columns
chart_guest_os_information_any_os.yaml Guest OS Information - any OS No operating_system.product_name, operating_system.service_pack, name, vendor_display, os_image_name, operating_system.version, operating_system.build_number, operating_system.product_key, operating_system.productid
chart_hosts_summary_by_version.yaml Hosts Summary No name, ipaddress, vmm_vendor_display, vmm_product, vmm_version, vmm_buildnumber
chart_number_of_nodes_per_cpu_cores.yaml Number of Nodes per CPU Cores No name, computer_system.hardware.cpu_total_cores
chart_pods_per_ready.yaml Pods per Ready Status No name, ready_condition_status
chart_vendor_and_guest_os.yaml Vendor and Guest OS No vendor_display, operating_system.product_name, name, operating_system.name
chart_virtual_infrastructure_platforms.yaml Virtual Infrastructure Platforms No vmm_vendor_display, vmm_product, operating_system.product_name, name
nodes_by_cpu_capacity.yaml Nodes By Capacity No name, computer_system.hardware.cpu_total_cores, computer_system.hardware.memory_mb
nodes_by_cpu_usage.yaml Nodes By CPU Usage Yes name,max_cpu_usage_rate_average_avg_over_time_period
nodes_by_memory_usage.yaml Nodes By Memory Usage Yes name, max_mem_usage_absolute_average_avg_over_time_period
pods_per_ready.yaml Pods per Ready Status No name,ready_condition_status
projects_by_cpu_usage.yaml Projects By CPU Usage Yes name, max_cpu_usage_rate_average_avg_over_time_period
projects_by_memory_usage.yaml Projects By Memory Usage Yes name, max_mem_usage_absolute_average_avg_over_time_period
projects_by_number_of_containers.yaml Projects by Number of Containers No name, containers_count
projects_by_number_of_pods.yaml Projects by Number of Pods No name, groups_count
recently_discovered_pods.yaml Recently Discovered Pods Yes name, ready_condition_status, ems_created_on
report_top_cpu_consumers_weekly.yaml Top CPU Consumers (weekly) Yes resource_name, ems_cluster.name, host.name, cpu_usage_rate_average__avg, cpu_usage_rate_average
report_top_memory_consumers_weekly.yaml Top Memory Consumers (weekly) Yes resource_name, ems_cluster.name, host.name, derived_memory_used__avg, derived_memory_used
report_top_storage_consumers.yaml Top Storage Consumers No name, ems_cluster.name, host.name, used_disk_storage, provisioned_storage
tenant_quotas.yaml Tenant Quotas No name, tenant_quotas.name, tenant_quotas.total, tenant_quotas.used, tenant_quotas.allocated, tenant_quotas.available

@kbrock
Copy link
Member Author

kbrock commented Mar 27, 2017

kicked - master is green now

@miq-bot
Copy link
Member

miq-bot commented Mar 27, 2017

Checked commit kbrock@0fa51b2 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
0 files checked, 0 offenses detected
Everything looks good. ⭐

@kbrock
Copy link
Member Author

kbrock commented Apr 7, 2017

ping @martinpovolny this work for you?

Copy link
Member

@isimluk isimluk left a comment

Choose a reason for hiding this comment

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

lgtm,

Just afraid, that copy-paste programmers will not copy-paste this one to other reports :-D

@kbrock
Copy link
Member Author

kbrock commented Apr 11, 2017

@isimluk would you prefer that we look at the columns brought back and detect a date column?

That part of detection is easy, it is detection in the :conditions option, that is a MiqQuery, that is potentially tricky

@isimluk
Copy link
Member

isimluk commented Apr 12, 2017

I am ok with this as it is. This is an improvement and I want to get it in. End of story. :-)

We have a huge pile of Work-in-progress. We should hire someone with MBA to tell us that is not good. :)

This is an improvement and I want to get it in.

@isimluk
Copy link
Member

isimluk commented Apr 12, 2017

Ping @martinpovolny, did you want to take a look also?

@kbrock
Copy link
Member Author

kbrock commented Apr 18, 2017

@martinpovolny this good for you?

@isimluk
Copy link
Member

isimluk commented Apr 18, 2017

These pr review are really a step back from where the things used to be. http://web.mit.edu/humor/Computers/real.programmers

@martinpovolny martinpovolny merged commit 03f378c into ManageIQ:master Apr 19, 2017
@martinpovolny
Copy link
Member

Sorry for taking so long. I failed to review my core review queue for some time :-(

@martinpovolny
Copy link
Member

@kbrock : fine yes/no?

@kbrock kbrock deleted the widget_time_sensitive_2 branch April 20, 2017 12:21
@kbrock
Copy link
Member Author

kbrock commented Apr 20, 2017

Thanks martin. Yes, we have other PRs in Fine, so marking as fine/yes

simaishi pushed a commit that referenced this pull request Apr 20, 2017
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit f482aee3f7d554c4098964dafc43761096594a1f
Author: Martin Povolny <mpovolny@redhat.com>
Date:   Wed Apr 19 09:13:53 2017 +0200

    Merge pull request #14497 from kbrock/widget_time_sensitive_2
    
    Mark widgets as non timezone sensitive
    (cherry picked from commit 03f378c89f43d3fcd10eedc015f0df7ad3367c45)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1251259

@kbrock
Copy link
Member Author

kbrock commented May 8, 2017

This is a followup to #14386 which has been backported to Euwe
I would like to backport this to E as well. Can we tack onto BZ from 14386?
thanks

@kbrock kbrock added the euwe/yes label May 8, 2017
@simaishi
Copy link
Contributor

@kbrock BZ from #14386 is already closed as release went out and can't be re-used...

@chessbyte chessbyte added this to the Sprint 59 Ending Apr 24, 2017 milestone Jun 7, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants