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 #30360: Drop Pulp 2 and Qpid #356

Closed
wants to merge 1 commit into from

Conversation

ehelms
Copy link
Member

@ehelms ehelms commented Jul 9, 2020

No description provided.

@ehelms
Copy link
Member Author

ehelms commented Jul 9, 2020

Related to theforeman/puppet-foreman_proxy_content#271 but does not have the same level of re-factoring impact since all Pulpcore configuration exists over in puppet-foreman_proxy_content.

@jlsherrill can you look and make sure the katello.yml is accurate?

@ekohl ekohl mentioned this pull request Jul 10, 2020
Comment on lines 20 to 22
Boolean $use_pulp_2_for_file = false,
Boolean $use_pulp_2_for_docker = false,
Boolean $use_pulp_2_for_yum = false,
Copy link
Member

Choose a reason for hiding this comment

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

These can be dropped as well if there's no Pulp 2.

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 agree they ought to be dropped, just waiting for confirmation from @jlsherrill that they have been dropped code-wise inside Katello.

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

yep +1 safe to drop

@@ -20,7 +20,6 @@
Boolean $use_pulp_2_for_file = false,
Boolean $use_pulp_2_for_docker = false,
Boolean $use_pulp_2_for_yum = false,
Stdlib::Absolutepath $repo_export_dir = '/var/lib/pulp/katello-export',
Copy link
Member

Choose a reason for hiding this comment

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

Please also drop the docs for this one

Comment on lines 7 to +10
:content_types:
:file: <%= @enable_file %>
:yum: <%= @enable_yum %>
:deb: <%= @enable_deb %>
:puppet: <%= @enable_puppet %>
:docker: <%= @enable_docker %>
:ostree: <%= @enable_ostree %>
:docker: <%= @enable_container %>
Copy link
Member

Choose a reason for hiding this comment

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

AFAIK content types are Pulp 2 only. With Pulp 3 they're discovered via capabilities in the Smart Proxy

Copy link
Contributor

Choose a reason for hiding this comment

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

these are technically still needed as this tells katello what nav items to show, and what type of repositories can be created. We could look into looking at the smart proxy for this, but aren't currently.

Copy link
Member

Choose a reason for hiding this comment

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

Can we open an issue for that to track? From the installer side we want as little configuration as possible and a more dynamic application would be the result.

@@ -24,7 +21,6 @@
:ssl_ca_file: <%= @candlepin_ca_cert %>

:pulp:
:url: <%= @pulp_url %>
:ca_cert_file: <%= @pulp_ca_cert %>

:use_pulp_2_for_content_type:
Copy link
Member

Choose a reason for hiding this comment

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

Feels like this could be removed as well

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -33,8 +29,3 @@
:yum: <%= @use_pulp_2_for_yum %>

:katello_applicability: <%= !@use_pulp_2_for_yum %>
Copy link
Contributor

Choose a reason for hiding this comment

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

this could just be set to 'true' for now

@ehelms ehelms closed this Nov 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants