-
Notifications
You must be signed in to change notification settings - Fork 6
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
Catch scc request timeout exception #63
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,12 @@ | ||
------------------------------------------------------------------- | ||
Thu Feb 8 16:10:29 UTC 2024 - Luis Caparroz <[email protected]> | ||
|
||
- Adds UI dialog to allow the user to retry the SCC credential validation when | ||
the request times out (bsc#1218084). | ||
- Adds HTTP User-Agent to requests to the SCC API and changes the enpoint for | ||
credential validation. | ||
- Version 1.3.5 | ||
|
||
------------------------------------------------------------------- | ||
Thu Jun 9 10:47:13 UTC 2022 - Dominique Leuenberger <[email protected]> | ||
|
||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -26,6 +26,8 @@ module RMT; end | |||||
class RMT::WizardSCCPage < Yast::Client | ||||||
include ::UI::EventDispatcher | ||||||
|
||||||
YAST_RMT_USER_AGENT = 'yast2-rmt'.freeze | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What about adding some version number to the agent string? Standard web browsers do that as well. Non-working pseudocode: YAST_RMT_USER_AGENT = "yast2-rmt/#{RMT::VERSION}".freeze In theory this could allow to handle some corner cases or bugs in SMT on the server size. Or if the SCC changes the API it could fallback to old API for old versions... Ideally this should not be needed but it would be nice to have this possibility as the last resort... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I created the constant module |
||||||
|
||||||
def initialize(config) | ||||||
textdomain 'rmt' | ||||||
@config = config | ||||||
|
@@ -126,14 +128,27 @@ def scc_credentials_valid? | |||||
) | ||||||
) | ||||||
|
||||||
uri = URI('https://scc.suse.com/connect/organizations/systems') | ||||||
uri = URI('https://scc.suse.com/connect/organizations/orders') | ||||||
req = Net::HTTP::Get.new(uri) | ||||||
req.basic_auth(@config['scc']['username'], @config['scc']['password']) | ||||||
|
||||||
res = Net::HTTP.start(uri.host, uri.port, use_ssl: true) { |http| http.request(req) } | ||||||
req['User-Agent'] = YAST_RMT_USER_AGENT | ||||||
|
||||||
valid_credentials = nil | ||||||
while valid_credentials.nil? | ||||||
begin | ||||||
res = Net::HTTP.start(uri.host, uri.port, use_ssl: true) { |http| http.request(req) } | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a minor suggestion:
Suggested change
This is more robust as it will still work after you change the URL above to plain HTTP. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's a nice enhancement, but since the URL is hard-coded, it will not be very helpful. I wonder if it would be valuable for test and development purposes to have a way to configure the SCC URL (e.g.: env var), then this change would be very welcome. I had to manually change both the URL and the |
||||||
valid_credentials = (res.code.to_i == 200) | ||||||
rescue Net::ReadTimeout | ||||||
break valid_credentials = false unless Popup.ErrorAnyQuestion( | ||||||
_('Request Timeout'), | ||||||
_("The request to SCC timed out.\n\nWould you like to try again?"), | ||||||
_('Retry'), _('Cancel'), :focus_yes | ||||||
) | ||||||
end | ||||||
end | ||||||
|
||||||
UI.CloseDialog | ||||||
|
||||||
res.code.to_i == 200 | ||||||
valid_credentials | ||||||
end | ||||||
end |
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.
Thank you very much Luis! This is awesome!