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

Fixes #32249 - Fix the error message "Unable to print debug information" #9290

Merged
merged 1 commit into from May 6, 2021

Conversation

lfu
Copy link
Member

@lfu lfu commented Apr 15, 2021

Parameter payload is in string format but treated as a hash which always throws an error.

…information"

Parameter payload is in string format but treated as a hash which always throws an error.
@theforeman-bot
Copy link

Issues: #32249

Comment on lines -69 to -70
# calling to_json on file has side-effects breaking manifest import.
# this fix prevents this problem
Copy link
Member

Choose a reason for hiding this comment

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

Is this still an issue?

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't think the comment is appropriate any more as the payload is either an empty hash or a json string.

Copy link
Member

Choose a reason for hiding this comment

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

@jeremylenz this should be good to merge your question has been resolved?

Copy link
Member

Choose a reason for hiding this comment

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

@lfu @chris1984 Is it a guarantee that payload is always one of those two types?

I did a search and found this:

response = self.post(path, {:import => File.new(path_to_file, 'rb')}, self.default_headers.except('content-type'))

which would call print_debug_info here with {:import => File.new(path_to_file, 'rb')} as the payload, meaning the value of one of the hash keys is a File.

So it seems to me like this edge case may still be valid.

To make sure, I'd want to test this change while importing a manifest, since that's the action that would call Katello::Resources::Candlepin::Owner.import.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nice catch!

Copy link
Member Author

Choose a reason for hiding this comment

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

14:01:54 rails.1   | 2021-04-23T14:01:54 [D|kat|38550c4b] Resource POST request: /candlepin/owners/nc_organization/imports/async, {:import=>#<File:/home/vagrant/foreman/tmp/import_1fd5c33d3f309aba8129.zip>}
14:01:54 rails.1   | 2021-04-23T14:01:54 [D|kat|38550c4b] Headers: {"accept":"application/json","accept-language":"en","X-Correlation-ID":"38550c4b-bc34-4ce4-b11d-8120c4cc8ce1","cp-user":"foreman_admin"}
14:01:54 rails.1   | 2021-04-23T14:01:54 [D|kat|38550c4b] Body: {"import":"#\u003cFile:0x0000000019a1fdd8\u003e"}
14:01:55 rails.1   | 2021-04-23T14:01:55 [D|kat|38550c4b] Candlepin request 9d8e6e31-ab3c-4d4b-99bd-9232f58ee50b returned with code 200
14:01:55 rails.1   | 2021-04-23T14:01:55 [D|kat|38550c4b] Processing response: 200
14:01:55 rails.1   | 2021-04-23T14:01:55 [D|kat|38550c4b] {"created":"2021-04-23T14:01:55+0000","updated":"2021-04-23T14:01:55+0000","id":"8a88e7de786572690178ff097ca513c1","name":"Import Manifest","group":null,"origin":"lucydevel.satellite.lab.eng.rdu2.redhat.com","executor":null,"principal":"foreman_admin","state":"CREATED","previousState":"CREATED","startTime":null,"endTime":null,"attempts":0,"maxAttempts":1,"statusPath":"/jobs/8a88e7de786572690178ff097ca513c1","resultData":null,"key":"ImportJob"}
14:01:57 rails.1   | 2021-04-23T14:01:57 [D|kat|38550c4b] Resource GET request: /candlepin/jobs/8a88e7de786572690178ff097ca513c1?
14:01:57 rails.1   | 2021-04-23T14:01:57 [D|kat|38550c4b] Headers: {"accept":"application/json","accept-language":"en","content-type":"application/json","X-Correlation-ID":"38550c4b-bc34-4ce4-b11d-8120c4cc8ce1","cp-user":"foreman_admin"}
14:01:57 rails.1   | 2021-04-23T14:01:57 [D|kat|38550c4b] Body: {}
14:01:57 rails.1   | 2021-04-23T14:01:57 [D|kat|38550c4b] Candlepin request d8d5350d-6139-47bc-bdb2-8cd656c5e784 returned with code 200
14:01:57 rails.1   | 2021-04-23T14:01:57 [D|kat|38550c4b] Processing response: 200
14:01:57 rails.1   | 2021-04-23T14:01:57 [D|kat|38550c4b] {"created":"2021-04-23T14:01:55+0000","updated":"2021-04-23T14:01:56+0000","id":"8a88e7de786572690178ff097ca513c1","name":"Import Manifest","group":null,"origin":"lucydevel.satellite.lab.eng.rdu2.redhat.com","executor":"lucydevel.satellite.lab.eng.rdu2.redhat.com","principal":"foreman_admin","state":"FINISHED","previousState":"RUNNING","startTime":"2021-04-23T14:01:55+0000","endTime":"2021-04-23T14:01:56+0000","attempts":1,"maxAttempts":1,"statusPath":"/jobs/8a88e7de786572690178ff097ca513c1","resultData":{"created":1619186516596,"updated":1619186516596,"id":"8a88e7de786572690178ff09827413c9","status":"SUCCESS","statusMessage":"nc_organization file imported successfully.","fileName":"import_1fd5c33d3f309aba8129.zip","generatedBy":"rhn-engineering-lfu","generatedDate":1618842552134,"upstreamConsumer":{"created":1619186516596,"updated":1619186516596,"id":"8a88e7de786572690178ff09827413ca","uuid":"64900532-f958-4915-b4c7-deeed9e46679","name":"lucy_sat_68","type":{"created":1618416173730,"updated":1618416173730,"id":"8a88e7de786572690178d11f02a2054d","label":"satellite","manifest":true},"ownerId":"8a88e7de786572690178efa8085512f8","contentAccessMode":null,"webUrl":"access.redhat.com/management/subscription_allocations/","apiUrl":"https://subscription.rhsm.redhat.com/subscription/consumers/"}},"key":"ImportJob"}

Copy link
Member

Choose a reason for hiding this comment

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

It's possible that the switch to Candlepin's async manifest import endpoint in #9145 has removed the need for this code. I'm not sure where the comment came from, or the details on the issue that it was causing. @jturel thoughts? (Maybe @chris1984 can also test some more manifest imports just so we're super-sure not to break anything.)

Copy link
Member

Choose a reason for hiding this comment

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

As long as manifest import isn't broken then I'm good :)

@chris1984
Copy link
Member

@lfu I am starting to test this, how do I make it so I get the error you are seeing, to see if the patch fixes it?

@chris1984 chris1984 self-assigned this Apr 19, 2021
@lfu
Copy link
Member Author

lfu commented Apr 19, 2021

@chris1984 Just check in any environment without this PR and with debug log level.
Message "Body: Unable to print debug information" is printed out pretty often.
Then verify with this PR applied and debug log level, that message should not show up any more.

Copy link
Member

@chris1984 chris1984 left a comment

Choose a reason for hiding this comment

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

ACK, I am not seeing the comment anymore after applying this patch.

@jturel
Copy link
Member

jturel commented May 6, 2021

@chris1984 @lfu go ahead and merge if this is ready :)

@chris1984 chris1984 merged commit ee5c736 into Katello:master May 6, 2021
@lfu lfu deleted the print_debug_info_7920 branch September 24, 2021 20:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants