Bug 1823991
Summary: | [RFE] Add a more performant way to sort reports | ||
---|---|---|---|
Product: | Red Hat Satellite | Reporter: | Shekhar Raut <sraut> |
Component: | Reporting | Assignee: | Marek Hulan <mhulan> |
Status: | CLOSED ERRATA | QA Contact: | Lukáš Hellebrandt <lhellebr> |
Severity: | medium | Docs Contact: | |
Priority: | medium | ||
Version: | 6.5.0 | CC: | bkearney, egolov, lhellebr, mhulan, nshaik, oprazak, saxon.mailey, sokeeffe, tbrisker |
Target Milestone: | 6.8.0 | Keywords: | FutureFeature, Reopened, Triaged |
Target Release: | Unused | ||
Hardware: | x86_64 | ||
OS: | Unspecified | ||
Whiteboard: | |||
Fixed In Version: | foreman-2.1.0-0 | Doc Type: | Enhancement |
Doc Text: | Story Points: | --- | |
Clone Of: | Environment: | ||
Last Closed: | 2020-10-27 13:01:20 UTC | Type: | Bug |
Regression: | --- | Mount Type: | --- |
Documentation: | --- | CRM: | |
Verified Versions: | Category: | --- | |
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |
Cloudforms Team: | --- | Target Upstream Version: | |
Embargoed: |
Comment 3
Shekhar Raut
2020-04-15 01:50:15 UTC
Hi Team, To provide further detail I came across this need whilst generating HTML reports. I was wanting to sort the output by host, but it would be ideal to be able to sort by multiple fields (eg domain,hostname) It seems that the output from the module load_hosts would need to include a sort option? Thanks in advance, Saxon After quick discussion, we will try to implement the following the load_* macros will support new keyword argument called order. The expected value should be a symbol or an array of symbols or hashes, like this load_hosts order: :ip load_hosts order: [ :ip, :name ] load_hosts order: [ :ip, :name => :desc, :mac => :desc ] keys (ip, name and mac) needs to be the SQL table column name. It will allow specifying and column of the table for ordering for a given model, Host in this case. Any concerns or comments on the suggestion? Hi Marek, That sounds like spot on what I am looking for :) is the table (or view) that the load_hosts module uses documented anywhere so I can have a look? Thanks again, Saxon There's no such documentation right now, but we plan to have generated documentation for all available macros soon and we can generate the list for this parameter quite easily then I believe. I hope it hits the same release :-) If not, I can give you the list of columns before we have it in the documentation. Thanks Marek, Much appreciated Created redmine issue https://projects.theforeman.org/issues/29578 from this bug I started looking at this, however I quickly realized, this won't work. The problem is, we load records in batches. That is necessary in case there's too many records in DB. Batch processing is necessary to avoid RAM depletion. But if we apply order, it would order one batch only. E.g. if we sort the list alphabetically (and assume batch size is 3), it would look like this batch 1 a g x batch 2 b k l batch 3 d o z The default batch size in reality is 1 000 and can be change in the loader macro. At this point, I think the best option is to use `sort` on stored data. That does not use SQL ordering and therefore is slower, but should work. So we may add a new macro like <% order_reported_data :name %> That would at least not work with AR instances but just reported strings. Also that would allow sorting by all columns. Hi Bryan Kearney, Perhaps you don't understand how the satellite reporting engine works? This tool is marketed as a reporting engine that has ERB functionality and can generate CSV, HTML and really any other type of report. Perhaps you can explain to me how "sort this in an external tool" when generating a HTML file with multiple tables in it? Up until the point of your reply I was very happy with the respose from Red Hat... Now I am dissappointed and feel that RedHat has not met my expectations. What is the escalation process here? we spend literally millions of dollars with RedHat a year and we can't even get a simple enhancement... Saxon Apologies Saxon, there was some confusion that I'm going to try and clear up. Currently, you can sort reporting using the `.sort` Ruby method - this will be documented under https://bugzilla.redhat.com/show_bug.cgi?id=1819150, planned for 6.8. I'm going to re-open this BZ and use it to track a more performant way to sort reports that Marek has spoken about above. Hopefully, that aligns with you expectations Saxon, please comment if not. And apologies again, we don't always get it right first time, I appreciate you escalating this. Saxon, I have the initial version ready for testing. If you're interested, please take a look at https://github.com/theforeman/foreman/pull/7620, there's a gif recording of how it is supposed to work. There's one limitation that I know about, in case the table is sorted by multiple columns, there's no way to chose different order per column. I don't see an easy and performant implementation for this. However I think the proposed implementation covers most of the use cases already. Please leave a feedback either in the linked PR or here. While the recording shows hardocded order in a template, given the suggested implementation, it's very easy to extend the report template in a way, that the order would be exposed to the user, who renders the template. Once the administrator defines the template input, called e.g. Order, with preconfigured list of columns, the only thing required would be to modify the report_render line to <%= report_render order: input('Order') %>. That means user could then select desired order in the form from predefined list of possibilities. It's possible to do the same for multiple columns ordering, it would only need a mapping of input value to the column definition, e.g. using case/when expression. I can make sure we document such example in product documentation. Upstream bug assigned to mhulan Upstream bug assigned to mhulan Hi Marek, I had a look at the pul request and looks good, but I'm just wondering how I can use that with HTML reports. I followed "Creating an HTML Report" @ https://www.redhat.com/en/blog/getting-started-satellite-65-reporting-engine when I started with the HTML email reports and I note that when generating a HTML report you don't call render_report at the end of the template. Thanks again, Saxon Saxon, the example you refer to predates the native support for HTML output. It demonstrates how you can create very custom HTML report however starting with Satellite 6.7, user can chose what format the output should render when render_report macro is used. One of the option is HTML. That is now available for every report template we ship in Satellite. The limiting factor is, that the HTML has very basic styling. There's currently no way customize CSS for it. I'm attaching the example output below. An easy workaround is to inject <style></style> tag e.g. in the beginning of the report template, the result won't be valid HTML but browsers will be able to deal with it. We could add a global setting for customizing CSS in generated reports. Do you have some thoughts or further suggestions on this? the example output: <html> <head> <title>User - Registered Users</title> <style>th { background-color: black; color: white; } table,th,td { border-collapse: collapse; border: 1px solid black; } </style> </head> <body> <table> <thead> <tr> <th>Login</th> <th>Firstname</th> <th>Lastname</th> <th>Email</th> <th>Last Login</th> <th>Auth source</th> </tr> </thead> <tbody> <tr> <td>admin</td> <td>Admin</td> <td>User</td> <td>root</td> <td>2020-05-21 14:21:10 +0200</td> <td>Internal</td> </tr> <tr> <td>ares</td> <td></td> <td></td> <td></td> <td>2020-03-04 10:13:11 +0100</td> <td>Internal</td> </tr> </tbody> </table> </body> </html> Hi Marek, I'm just upgrading to 6.7 as we speak and will check this out and let you know. Saxon Hi Marek, I'm up and running on 6.7 now and have adjusted all my reporting to use report format option rather than hard-coding html into the template. (I like this, its easy to use/automate and it looks good when emailed) For the benefit of others this is a snippit of the code i use to generate reports from cron: ----- hammer report-template generate --name "Subscriptions" --report-format html > $HTMLFILE hammer report-template generate --name "Subscriptions" --report-format csv > $CSVFILE mutt -e 'set content_type=text/html' -s "$SUBJECT" $EMAILTO -a $CSVFILE -- < $HTMLFILE ----- Code and gif look good, like the idea of using an input rather than a hardcoding. With regard to not being able to use different order per column sorting multiple columns - I don't think that is a big issue - what you have put together should be fine for 99% of applications :) With regards to hard coded css, some thoughts; - Leave hard-coded default and add an input to override the style block? (not sure of character limits) - Add a tab for style and allow people to edit (be good to have a "revert to default button") - allow a url to be used to reference an external css? - not sure if this can be added to the template code block somehow? Thanks heaps, Saxon Thanks for the feedback, that is really helpful! I would have follow up questions regarding the css. Do you expect the need to set the different styling per report type or even per report rendering? In the first case, custom css could be specified on report templat level. The second would mean, user would need to pass it as a parameter for every rendering. Or is that more like one styling per one Satellite. Or perhaps by Satellite organization. All of what you suggest is implementable. I think each solution fits different use case. So I'd like to hear, how do you plan to use the customization. Given native HTML report is a new thing in 6.7, I assume we'll hear from more customers about their use cases soon too. Moving this bug to POST for triage into Satellite 6 since the upstream issue https://projects.theforeman.org/issues/29578 has been resolved. This was merged upstream and cherrypicked to Foreman 2.1 ack Verified with Sat 6.8 snap 5.0. The report items can now be sorted by using "report_render order: 'Column', reverse_order: true". This works correctly and ~instantly for 1500 items. Since the problem described in this bug report should be resolved in a recent advisory, it has been closed with a resolution of ERRATA. For information on the advisory (Important: Satellite 6.8 release), and where to find the updated files, follow the link below. If the solution does not work for you, open a new bug report. https://access.redhat.com/errata/RHSA-2020:4366 |