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

Add ability to generate report without uploading #850

Merged
merged 1 commit into from
Nov 1, 2023

Conversation

chris1984
Copy link
Member

@ShimShtein here is the updated PR

@chris1984 chris1984 requested a review from ShimShtein October 23, 2023 18:58
@chris1984 chris1984 force-pushed the reportupload branch 4 times, most recently from 2c06b6d to 4ed3044 Compare October 23, 2023 20:14
@chris1984
Copy link
Member Author

chris1984 commented Oct 23, 2023

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 chris1984 force-pushed the reportupload branch 3 times, most recently from 49424b9 to 97802c6 Compare October 25, 2023 17:42
Copy link
Member

@ShimShtein ShimShtein left a comment

Choose a reason for hiding this comment

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

@chris1984
Copy link
Member Author

chris1984 commented Oct 26, 2023

@chris1984
Copy link
Member Author

chris1984 commented Oct 26, 2023

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

@chris1984
Copy link
Member Author

@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
Copy link
Member

@chris1984
Copy link
Member Author

@jameerpathan111
Copy link
Contributor

@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 ShimShtein merged commit 45d5b1e into theforeman:foreman_3_9 Nov 1, 2023
2 checks passed
@ShimShtein
Copy link
Member

Thanks @jameerpathan111 and @chris1984 !
Merged.

@chris1984
Copy link
Member Author

@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

ShimShtein pushed a commit to ShimShtein/foreman_rh_cloud that referenced this pull request Nov 1, 2023
ShimShtein added a commit that referenced this pull request Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants