Unconditionally seed all standard reports and widgets #16062

yrudman · 2017-09-27T21:42:35Z

Before: seeding process did not update record in miq_report table if timestamp of yaml file in products/... directory older than in miq_report. (assumption was: newer file are always from later version).

Issue: files in cfme build 5.7.4.0 have newer date than in build 5.8.1.5 and report modified in 5.8.1.5 (in #14646) was not seeded when upgrading from 5.7.4.0 to 5.8.1.5

https://bugzilla.redhat.com/show_bug.cgi?id=1494819

After: Unconditionally seed all standard reports

\cc @gtanzillo

lpichler · 2017-09-27T23:03:58Z

this also fixes case from BZ, which was not reproduced => BZ Closed. (not need to reopen)
https://bugzilla.redhat.com/show_bug.cgi?id=1479883

but in the end we found how to reproduce it (example):
appliance 5.8.0 - created on Aug 1
appliance 5.8.1 - created on Aug 4
backup db from appliance 5.8.1 and restore it on appliance 5.8.0. (on Aug 4)
then we will have in db (rec.file_mtime.utc) Aug 4

rec.filename && (rec.file_mtime.nil? || rec.file_mtime.utc < rpt[:file_mtime])

but report yml file system is created Aug 1

rec.filename && (rec.file_mtime.nil? || `Aug 4` < 'Aug 1')

so it will not update the report and will cause problem with changed name of report (between 5.8.1 and 5.8.0)

not sure if this can happen in production.

thanks @yrudman

yrudman · 2017-09-28T12:49:47Z

@miq-bot add-label bug, reporting, fine/yes

miq-bot · 2017-09-28T12:50:02Z

@yrudman Cannot apply the following label because they are not recognized: cloud intel/reporting

…r patch on new version could be older that build date of older version, and skipping file based on timestamp will lead to keeping records from old version https://bugzilla.redhat.com/show_bug.cgi?id=1494819

… may lead to keep recotrds from older version (when timestamp on yaml file from older version is more resent) https://bugzilla.redhat.com/show_bug.cgi?id=1494819

miq-bot · 2017-09-28T20:52:52Z

Checked commits yrudman/manageiq@0b7e075~...ce79a8a with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
2 files checked, 0 offenses detected
Everything looks fine. 👍

jrafanie · 2017-09-28T21:53:01Z

app/models/miq_report/seeding.rb

-        if rec.filename && (rec.file_mtime.nil? || rec.file_mtime.utc < rpt[:file_mtime])
-          _log.info("#{typ.titleize}: [#{rec.name}] file has been updated on disk, synchronizing with model")
+        if rec.filename
+          _log.info("#{typ.titleize}: [#{rec.name}] file exists, synchronizing with model")


Ok, so I'm fine with these changes but I don't know if anything will be changed when we're constantly touching the report objects. For example, do we have report definitions sorted in the UI by updated on?

I don't think so but it might be worth running this through several server boots and verifying the report pages show the reports in the same order in some screens.

sorting does changes slightly between server reboots

@yrudman How is the sorting changing? Can you tell f that is a result of this change?

i think changing sorting is result of change, since before change reports record on DB was not touched.
Note: there is different sorting on left part (in tree) and on right part (in table)

before: (on master):

after (on this branch):
reboot 1 (red marks difference between rebootes):

reboot 2)

@yrudman can you find how they're sorting these?

@jrafanie I think there is no explicit sorting, also it does look inconsistent now when on the same screen there are same lists side by side, and those list have different order.
I think it make sense to sort those list of reports in the same way alphabetically.
Are you thinking of changing sorting in this PR ?

I am still debugging to find-out how exactly sorting done on UI side. At this point I can say how right side sorted - it is sorted by yaml file name (SELECT "miq_reports".* FROM "miq_reports" ORDER BY "miq_reports"."rpt_type" ASC, "miq_reports"."filename" ASC, "miq_reports"."name" ASC) and report with filename 005_VMs with Free Space _ 50% by Department will be first (which is shown in right side of screenshots.

It looks like the left side in report tree formed by using this SQL: SELECT "miq_reports".* FROM "miq_reports" WHERE "miq_reports"."name" IN ('VMs with Free Space > 50% by Department', 'VMs w/Free Space > 75% by Function', 'VMs w/Free Space > 75% by LOB', 'VM Disk Usage', 'Hardware Information for VMs', 'Vendor and Type', 'Vendor and Guest OS', 'VM Location and Size', 'VM UUIDs', 'VMs with no UUID', 'VMs with Volume Free Space <= 20%', 'VMs with Volume Free Space >= 80%', 'VMs by MAC Address', 'Unregistered VMs', 'Orphaned VMs', 'VMs Snapshot Summary', 'User Accounts - Windows', 'User Accounts - Linux', 'Account Groups - Windows', 'Account Groups - Linux', 'Guest OS Information - any OS', 'Guest OS Information - Windows', 'Guest OS Information - Linux', 'Guest OS Password Information - Windows', 'Guest OS HKLM Registry Information')
and this SQL do not have any explicit sorting. I did not find (yet) any sorting on UI side

@yrudman Can you confirm the order of these reports change on master without your changes? Based on your findings of the sql queries, I'd guess this would happen without your changes.

not sure, i've tried couple time and master still the same. I think the order without explicit ordering is not guaranteed, but since data was not changed with reboot it is the same, will try more on master ...

@jrafanie I was wrong in above comment - sorting slightly changing on master too

jrafanie · 2017-09-28T21:53:36Z

app/models/miq_widget.rb

@@ -472,7 +472,7 @@ def self.sync_from_hash(attrs)

    widget = find_by(:description => attrs["description"])
    if widget
-      if filename && widget.updated_at.utc < File.mtime(filename).utc
+      if filename


Same as above. I don't think this will break anything but it's been a while since I looked at how we query reports and widgets to know if we ever sort by updated_on

It looks like list of widget sorted alphabetically and not changing between server reboots

yrudman · 2017-10-02T15:00:16Z

@gtanzillo @jrafanie created issue about not consistent sorting of reports: #16083

Unconditionally seed all standard reports and widgets (cherry picked from commit 9653a69) https://bugzilla.redhat.com/show_bug.cgi?id=1497817

simaishi · 2017-10-02T18:41:23Z

Fine backport details:

$ git log -1
commit 991fea9ca2614879e0d435454588175cf83c688c
Author: Gregg Tanzillo <[email protected]>
Date:   Mon Oct 2 14:18:40 2017 -0400

    Merge pull request #16062 from yrudman/allow-to-change-report-name
    
    Unconditionally seed all standard reports and widgets
    (cherry picked from commit 9653a69061d76bf63a233778d6b715baa5e3a7c4)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1497817

…-name Unconditionally seed all standard reports and widgets (cherry picked from commit 9653a69) https://bugzilla.redhat.com/show_bug.cgi?id=1497817

miq-bot added bug fine/yes labels Sep 28, 2017

yrudman added 2 commits September 28, 2017 16:48

unconditionally seed all standard reports. Sometimes date of build fo…

0b7e075

…r patch on new version could be older that build date of older version, and skipping file based on timestamp will lead to keeping records from old version https://bugzilla.redhat.com/show_bug.cgi?id=1494819

unconditionally seed widgets, skeeping seed based on file's timestamp…

ce79a8a

… may lead to keep recotrds from older version (when timestamp on yaml file from older version is more resent) https://bugzilla.redhat.com/show_bug.cgi?id=1494819

yrudman force-pushed the allow-to-change-report-name branch from 144c110 to ce79a8a Compare September 28, 2017 20:49

yrudman changed the title ~~Unconditionally seed all standard reports.~~ Unconditionally seed all standard reports and widgets Sep 28, 2017

simaishi added the blocker label Sep 28, 2017

jrafanie reviewed Sep 28, 2017

View reviewed changes

miq-bot added the core/reporting label Sep 28, 2017

jrafanie reviewed Sep 28, 2017

View reviewed changes

gtanzillo self-assigned this Oct 2, 2017

gtanzillo added this to the Sprint 70 Ending Oct 2, 2017 milestone Oct 2, 2017

gtanzillo merged commit 9653a69 into ManageIQ:master Oct 2, 2017

yrudman deleted the allow-to-change-report-name branch October 2, 2017 18:20

simaishi pushed a commit that referenced this pull request Oct 2, 2017

Merge pull request #16062 from yrudman/allow-to-change-report-name

991fea9

Unconditionally seed all standard reports and widgets (cherry picked from commit 9653a69) https://bugzilla.redhat.com/show_bug.cgi?id=1497817

simaishi added fine/backported and removed fine/yes labels Oct 2, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unconditionally seed all standard reports and widgets #16062

Unconditionally seed all standard reports and widgets #16062

yrudman commented Sep 27, 2017

lpichler commented Sep 27, 2017

yrudman commented Sep 28, 2017 •

edited

Loading

miq-bot commented Sep 28, 2017

miq-bot commented Sep 28, 2017

jrafanie Sep 28, 2017

yrudman Sep 28, 2017

gtanzillo Sep 29, 2017

yrudman Sep 29, 2017

jrafanie Sep 29, 2017

yrudman Sep 29, 2017

yrudman Sep 29, 2017 •

edited

Loading

jrafanie Sep 29, 2017

yrudman Sep 29, 2017

yrudman Sep 29, 2017

jrafanie Sep 28, 2017

yrudman Sep 28, 2017

yrudman commented Oct 2, 2017

simaishi commented Oct 2, 2017

Unconditionally seed all standard reports and widgets #16062

Unconditionally seed all standard reports and widgets #16062

Conversation

yrudman commented Sep 27, 2017

lpichler commented Sep 27, 2017

yrudman commented Sep 28, 2017 • edited Loading

miq-bot commented Sep 28, 2017

miq-bot commented Sep 28, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yrudman Sep 29, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yrudman commented Oct 2, 2017

simaishi commented Oct 2, 2017

yrudman commented Sep 28, 2017 •

edited

Loading

yrudman Sep 29, 2017 •

edited

Loading