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

Remove tower_legacy module_utils that appears unused #14421

Merged
merged 2 commits into from
Feb 16, 2024

Conversation

AlanCoding
Copy link
Member

@AlanCoding AlanCoding commented Sep 6, 2023

SUMMARY

This does stuff like processing the config file... but that is already done in the main controller_api.py module utils file.

🗑️

NOTE: This requires bumping the major version of AWX, which I'm 💯 cool with

ISSUE TYPE
  • Bug, Docs Fix or other nominal change
COMPONENT NAME
  • Collection

@github-actions github-actions bot added the component:awx_collection issues related to the collection for controlling AWX label Sep 6, 2023
@relrod relrod changed the title Remove tower_legacy module that appears unused Remove tower_legacy module_utils that appears unused Sep 6, 2023
@relrod relrod changed the title Remove tower_legacy module_utils that appears unused Remove tower_legacy module_utils that appears unused Sep 6, 2023
@relrod
Copy link
Member

relrod commented Sep 6, 2023

@AlanCoding Assuming TowerLegacyModule still actually works, I think it be better to have a deprecation cycle here.

module_utils are public facing API (people could in theory write custom modules using our module_utils and if this suddenly goes away, we break them with no warning).

ansible-core has built-in handling for deprecation warnings, we just need to emit them in the output.

So in the TowerLegacyModule we define, we could call self.deprecate(...) in the overloaded constructor. In the other functions, we take the module as a parameter, so we can module.deprecate(...).

If this doesn't actually work at all anymore, then probably doesn't matter as much.

@AlanCoding
Copy link
Member Author

So in the TowerLegacyModule we define, we could call self.deprecate(...) in the overloaded constructor.

What do you mean "in the overloaded constructor"? As you say, warning is a self. call. IMO the most likely way someone will get burned by this is if they had an unused import (similar to what our test did), but the deprecation machinery gives us no way to warn about that.

@relrod
Copy link
Member

relrod commented Sep 6, 2023

What do you mean "in the overloaded constructor"?

I was just saying, we already overload the constructor, we can just call deprecate() there.

For the import issue... yeah that's a concern. I'm not sure of a good way around that. :(

@AlanCoding
Copy link
Member Author

^ we could, I just don't think it's worth the effort. I'd rather merge this as it is.

@AlanCoding AlanCoding merged commit 1f7be92 into ansible:devel Feb 16, 2024
21 checks passed
djyasin pushed a commit to djyasin/awx that referenced this pull request Sep 16, 2024
* Remove tower_legacy module that appears unused

* Update license details
djyasin pushed a commit to djyasin/awx that referenced this pull request Nov 11, 2024
* Remove tower_legacy module that appears unused

* Update license details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:awx_collection issues related to the collection for controlling AWX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants