Remove remaining subscription attachment references #20194
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Since SCA (Simple Content Access) is now the default mode for all organizations, subscription attachment to activation keys and hosts is no longer needed or possible. This completes the cleanup started in PR SatelliteQE#19931. Changes: - Remove activationkey_add_subscription_to_repo() helper (unused) - Remove ActivationKey.add_subscription(), remove_subscription(), subscriptions() - Remove Host.subscription_attach() and subscription_remove() - Remove stubbed test_positive_candlepin_events_processed_by_STOMP - Remove is_sca_mode_enabled() references from helpers and tests - Remove conditional subscription attachment code from setup helpers - Clean up subscription attachment checks from test_sca_end_to_end 12 files changed, 5 insertions(+), 228 deletions(-)
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey there - I've reviewed your changes - here's some feedback:
- Now that Simple Content Access is always enabled, consider removing the is_sca_mode_enabled method and any related configuration flags entirely to eliminate leftover conditional logic.
- With all subscription‐attach code gone, you can simplify setup_activation_key and setup_content helpers by dropping unused parameters and branches, streamlining their activation key creation flow.
- Add a test to confirm that attempts to use the removed subscription attach/remove CLI commands now fail or are unrecognized, ensuring the user‐facing behavior is as expected.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Now that Simple Content Access is always enabled, consider removing the is_sca_mode_enabled method and any related configuration flags entirely to eliminate leftover conditional logic.
- With all subscription‐attach code gone, you can simplify setup_activation_key and setup_content helpers by dropping unused parameters and branches, streamlining their activation key creation flow.
- Add a test to confirm that attempts to use the removed subscription attach/remove CLI commands now fail or are unrecognized, ensuring the user‐facing behavior is as expected.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
vsedmik
approved these changes
Nov 6, 2025
Contributor
vsedmik
left a comment
vsedmik
left a comment
There was a problem hiding this comment.
One question bellow, otherwise looks good to me!
Comment on lines
+2484
to
+2485
| # refresh repository metadata on the host | ||
| rhel_contenthost.execute('subscription-manager repos --list') |
Contributor
There was a problem hiding this comment.
I think we don't need this, but I don't mind keeping it either.
LadislavVasina1
approved these changes
Nov 7, 2025
Contributor
LadislavVasina1
left a comment
LadislavVasina1
left a comment
There was a problem hiding this comment.
Changes look good to me, thanks @jeremylenz
jameerpathan111
requested changes
Nov 7, 2025
Addresses PR review feedback by removing all remaining references to subscription attachment methods that were removed in the initial PR. This includes removing calls to ActivationKey.subscriptions() and Host.subscription_attach() in tests, removing subscription attachment API endpoints from the API test file, and cleaning up the unused rh_subscriptions parameter from helper methods since subscription attachment is no longer supported in SCA mode. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Contributor
Author
|
Opened SatelliteQE/nailgun#1377 |
Contributor
Author
|
@jameerpathan111 updated |
ColeHiggins2
approved these changes
Nov 10, 2025
Member
ColeHiggins2
left a comment
ColeHiggins2
left a comment
There was a problem hiding this comment.
im assuming you double checked that any usage for any removed method or functions was updated or caused no failures? maybe we can run a PRT job on subscriptions or activation keys and make sure tests are passing at a reasonable percentage?
Member
|
trigger: test-robottelo |
Collaborator
|
PRT Result |
Satellite-QE
added
the
PRT-Passed
jameerpathan111
approved these changes
Nov 11, 2025
Contributor
|
@jeremylenz thanks for raising this PR and addressing the requested changes. It should fix many test failures. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Problem Statement
Following PR #19931 which removed auto-attach, autoheal, and simple content access references, there were still remaining references to subscription attachment functionality throughout the codebase. Since SCA (Simple Content Access) is now the default mode for all organizations, subscription attachment to activation keys and hosts is no longer needed or possible.
Solution
This PR completes the cleanup by removing all remaining subscription attachment references:
Phase 1: Removed dead code and obsolete parameters
activationkey_add_subscription_to_repo()helper method from cli_factory.py (unused)auto_attach=Falseparameters from test_rhcloud_iop.py (redundant in SCA mode)test_positive_candlepin_events_processed_by_STOMP(never implemented)Phase 2: Removed CLI wrapper methods
ActivationKey.add_subscription(),remove_subscription(), andsubscriptions()Host.subscription_attach()andsubscription_remove()subscription_register()andsubscription_unregister()(still needed for registration)Phase 3: Cleaned up tests
test_sca_end_to_endPhase 4: is_sca_mode_enabled cleanup
is_sca_mode_enabled()references from helper methods and testsFiles changed: 12 files changed, 5 insertions(+), 228 deletions(-)
Related Issues
Related to PR #19931