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

chargeback - group results with unknown project under 'unknown project' #14811

Merged
merged 1 commit into from Apr 19, 2017

Conversation

zeari
Copy link

@zeari zeari commented Apr 19, 2017

BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1442158
containers without an identifiable project will grouped under "Unknown Project" in Chargeback for container images reports
screencapture-localhost-3000-report-explorer-1492607823456

@gtanzillo @lpichler @isimluk Please review
cc @simon3z

@zeari
Copy link
Author

zeari commented Apr 19, 2017

@miq-bot add_label chargeback, providers/containers

@miq-bot
Copy link
Member

miq-bot commented Apr 19, 2017

Checked commit zeari@86e8b82 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
1 file checked, 0 offenses detected
Everything looks good. 🍪

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.

👍 This will work.

Good idea with naming it unknown and showing it to the user.

Copy link
Member

@gtanzillo gtanzillo left a comment

Choose a reason for hiding this comment

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

This seems like it should address the case where a container image is not connected a project. Were you able to test this out to make sure the the "unknown project" can be displayed properly on the report?

@isimluk isimluk self-assigned this Apr 19, 2017
@isimluk
Copy link
Member

isimluk commented Apr 19, 2017

For the future reference, the traceback was:

[NoMethodError]: undefined method `id' for nil:NilClass  Method:[rescue in _async_generate_table]
[----] E, ERROR -- : /var/www/miq/vmdb/app/models/chargeback_container_image.rb:61:in `default_key'
/var/www/miq/vmdb/app/models/chargeback.rb:32:in `report_row_key'
/var/www/miq/vmdb/app/models/chargeback.rb:11:in `block in build_results_for_report_chargeback'
/var/www/miq/vmdb/app/models/chargeback/consumption_history.rb:31:in `block (2 levels) in for_report'
/var/www/miq/vmdb/app/models/chargeback/consumption_history.rb:27:in `each'
/var/www/miq/vmdb/app/models/chargeback/consumption_history.rb:27:in `block in for_report'
/var/www/miq/vmdb/app/models/chargeback/consumption_history.rb:13:in `each'
/var/www/miq/vmdb/app/models/chargeback/consumption_history.rb:13:in `each_cons'
/var/www/miq/vmdb/app/models/chargeback/consumption_history.rb:13:in `for_report'
/var/www/miq/vmdb/app/models/chargeback.rb:8:in `build_results_for_report_chargeback'
/var/www/miq/vmdb/app/models/chargeback_container_image.rb:55:in `build_results_for_report_ChargebackContainerImage'
/var/www/miq/vmdb/app/models/miq_report/generator.rb:198:in `_generate_table'
/var/www/miq/vmdb/app/models/miq_report/generator.rb:172:in `block in generate_table’

@simon3z
Copy link
Contributor

simon3z commented Apr 19, 2017

@gtanzillo instead of creating something custom for this entry can the reporting system cope with nils? Maybe transforming those in special vaules e.g. "Missing Field".

@zeari
Copy link
Author

zeari commented Apr 19, 2017

This seems like it should address the case where a container image is not connected a project. Were you able to test this out to make sure the the "unknown project" can be displayed properly on the report?

@gtanzillo Yes. The screenshot above is the result of running the report after setting the project_id to nil on a pod(container_group)

@isimluk isimluk merged commit d573adc into ManageIQ:master Apr 19, 2017
@gtanzillo
Copy link
Member

Thanks @zeari I forgot to look at the screenshot! 👍
@simon3z

@gtanzillo instead of creating something custom for this entry can the reporting system cope with nils? Maybe transforming those in special values e.g. "Missing Field".
I think the reporting system will handle a grouping of nil without blowing up. But, I think its better that there is an actual name on the grouping because the reporting system will just show the nil.

@simon3z
Copy link
Contributor

simon3z commented Apr 19, 2017

I think the reporting system will handle a grouping of nil without blowing up. But, I think its better that there is an actual name on the grouping because the reporting system will just show the nil.

@gtanzillo if the column is named "Project" (as in this case) and the cell has a value of "Missing Field" (when nil) then it seems descriptive enough.

@isimluk
Copy link
Member

isimluk commented Apr 19, 2017

I think there might be some confusion going on. The project AR does never get down to the report formatter. The project AR never leaves the ChargebackContainerProject class.

simaishi pushed a commit that referenced this pull request Apr 20, 2017
chargeback - group results with unknown project under 'unknown project'
(cherry picked from commit d573adc)
@simaishi
Copy link
Contributor

Fine backport details:

$ git log -1
commit 005d38df9ece2325a7c9ef3419aa29f1932b0b1b
Author: Šimon Lukašík <isimluk@fedoraproject.org>
Date:   Wed Apr 19 16:14:37 2017 +0200

    Merge pull request #14811 from zeari/fix_nil_project_cb_report
    
    chargeback - group results with unknown project under 'unknown project'
    (cherry picked from commit d573adcf7585e17fdcc961912303a27e2c66ee65)

@isimluk isimluk added this to the Sprint 59 Ending Apr 24, 2017 milestone Apr 20, 2017
@chessbyte chessbyte added the bug label Apr 26, 2017
simaishi pushed a commit that referenced this pull request Apr 26, 2017
chargeback - group results with unknown project under 'unknown project'
(cherry picked from commit d573adc)

https://bugzilla.redhat.com/show_bug.cgi?id=1444041
@simaishi
Copy link
Contributor

Euwe backport details:

$ git log -1
commit 834739f2b83494293f89241efde3b8ebd1706e06
Author: Šimon Lukašík <isimluk@fedoraproject.org>
Date:   Wed Apr 19 16:14:37 2017 +0200

    Merge pull request #14811 from zeari/fix_nil_project_cb_report
    
    chargeback - group results with unknown project under 'unknown project'
    (cherry picked from commit d573adcf7585e17fdcc961912303a27e2c66ee65)
    
    https://bugzilla.redhat.com/show_bug.cgi?id=1444041

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.

None yet

7 participants