theforeman / foreman_rh_cloud

a plugin to Foreman that generates and uploads reports to the Red Hat cloud
GNU General Public License v3.0
6 stars 32 forks source link

Add ability to generate report without uploading #850

Closed chris1984 closed 1 year ago

chris1984 commented 1 year ago

@ShimShtein here is the updated PR

chris1984 commented 1 year ago

Are the tests failing because of my new param and the conditional I put on the job?

Looking at log/test.log I see this

2023-10-23T16:55:07 [I|app|] InventoryUpload::Api::InventoryControllerTest: test_0001_Starts report generation
2023-10-23T16:55:07 [I|app|] ---------------------------------------------------------------------------------
2023-10-23T16:55:07 [I|app|] Processing by Api::V2::RhCloud::InventoryController#generate_report as JSON
2023-10-23T16:55:07 [I|app|]   Parameters: {"organization_id"=>"447626459", "inventory"=>{}}
2023-10-23T16:55:07 [D|tax|] Current location set to none
2023-10-23T16:55:07 [D|tax|] Current organization set to org11
2023-10-23T16:55:07 [I|app|] Completed 500 Internal Server Error in 4ms (ActiveRecord: 0.7ms | Allocations: 1881)
2023-10-23T16:55:07 [D|tax|] Current location set to none
2023-10-23T16:55:07 [D|tax|] Current organization set to none
chris1984 commented 1 year ago

You have missed a reference in https://github.com/theforeman/foreman_rh_cloud/blob/foreman_3_9/lib/foreman_inventory_upload/async/generate_all_reports_job.rb#L36 and https://github.com/theforeman/foreman_rh_cloud/blob/foreman_3_9/lib/tasks/rh_cloud_inventory.rake#L50 Besides that - looks good.

Added the reference to disconnected on the genereate_all_reports_job.rb, the rake file already has the change you requested:

https://github.com/theforeman/foreman_rh_cloud/pull/850/files#diff-9ca8d337c7041b111f4dee13b8c59910e8ad72ca97ca82b0a3b845f68893555aR51

chris1984 commented 1 year ago

@jameerpathan111 Can you test this change with CURL? Going to work on the hammer PR while awaiting your tests

chris1984 commented 1 year ago

@jameerpathan111

It looks like you can use hammer if you want instead of CURL, it picked up the new param with this pr checked out

[vagrant@hammer ~]$ hammer rh_report generate -h
Usage:
    hammer rh_report generate [OPTIONS]

Options:
 --disconnected BOOLEAN                  Generate the report, but do not upload
 --location[-id|-title] VALUE/NUMBER     Set the current location context for the request
 --organization[-id|-title] VALUE/NUMBER Set the current organization context for the request
 -h, --help                              Print help
ShimShtein commented 1 year ago

We have missed a reference: https://github.com/theforeman/foreman_rh_cloud/blob/5745be80c865dad235c6c6057118ac8b1d26a8ab/lib/foreman_inventory_upload/async/generate_report_job.rb#L16C3-L17C31 should have the disconnected parameter passed too

chris1984 commented 1 year ago

We have missed a reference: https://github.com/theforeman/foreman_rh_cloud/blob/5745be80c865dad235c6c6057118ac8b1d26a8ab/lib/foreman_inventory_upload/async/generate_report_job.rb#L16C3-L17C31 should have the disconnected parameter passed too

@ShimShtein updated

jameerpathan111 commented 1 year ago

@chris1984 @ShimShtein I was able to use api to generate a new report and also performed basic sanity checks on generated report. But I am not seeing rh_report subcommand in hammer, not sure why. I tried foreman-rake apipie:cache but it didn't help either.

ShimShtein commented 1 year ago

Thanks @jameerpathan111 and @chris1984 ! Merged.

chris1984 commented 1 year ago

@chris1984 @ShimShtein I was able to use api to generate a new report and also performed basic sanity checks on generated report. But I am not seeing rh_report subcommand in hammer, not sure why. I tried foreman-rake apipie:cache but it didn't help either.

When this goes into a snap, we can test it with hammer and see if we see it. I spun up a centos7-hammer-devel box from forklift and it saw it right away, so maybe something was up with hammer on your box