-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add ability to generate report without uploading #850
Conversation
2c06b6d
to
4ed3044
Compare
Are the tests failing because of my new param and the conditional I put on the job? Looking at 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 |
49424b9
to
97802c6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
97802c6
to
5745be8
Compare
Added the reference to disconnected on the |
@jameerpathan111 Can you test this change with CURL? Going to work on the hammer PR while awaiting your tests |
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 |
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 |
5745be8
to
65a9d44
Compare
@ShimShtein updated |
65a9d44
to
1d824f8
Compare
1d824f8
to
5107be3
Compare
@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 |
Thanks @jameerpathan111 and @chris1984 ! |
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 |
Co-authored-by: Chris Roberts <[email protected]>
@ShimShtein here is the updated PR