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

refs #37923 - Bug fixes & improvements for the Default HTTP proxy #11266

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

stejskalleos
Copy link
Contributor

@stejskalleos stejskalleos commented Dec 18, 2024

Follow up on #11183 with

  • Unified naming of the parameter ('content_default_http_proxy')
  • On edit form showing information if the capsule is default
  • Fixed API + extended show with 'content_default_http_proxy'

Follow up on PR#11183 with:
* Unified naming of the parameter ('content_default_http_proxy')
* On edit form showing information if the capsule is default
* Fixed API + extended show with 'content_default_http_proxy'
@stejskalleos
Copy link
Contributor Author

@jeremylenz review please
cc @Gauravtalreja1

@stejskalleos
Copy link
Contributor Author

@ianballou hi, anyone available for a review?

@ianballou
Copy link
Member

I'll double check that this is on our to-do list on Monday

param :http_proxy, Hash do
param :default_content_proxy, :bool, :required => false, :desc => N_('Set this HTTP proxy as the default content HTTP proxy')
param :content_default_http_proxy, :bool, :required => false, :desc => N_('Set this HTTP proxy as the default content HTTP proxy')
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
param :content_default_http_proxy, :bool, :required => false, :desc => N_('Set this HTTP proxy as the default content HTTP proxy')
param :default_content_http_proxy, :bool, :required => false, :desc => N_('Set this HTTP proxy as the default content HTTP proxy')

Should this be the name to be consistent with the UI option?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The setting has the key content_default_http_proxy, so I'd like to keep it in sync with it.

Copy link
Member

@ianballou ianballou left a comment

Choose a reason for hiding this comment

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

Working well!

@ianballou
Copy link
Member

Test failures are unrelated, merging.

@ianballou ianballou merged commit b316f4c into Katello:master Jan 13, 2025
24 of 26 checks passed
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