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

Pull the correct collection plugin for the product #15658

Merged
merged 16 commits into from
Dec 16, 2024

Conversation

jessicamack
Copy link
Member

@jessicamack jessicamack commented Nov 22, 2024

SUMMARY

There exist several cases of inventory plugins with the same name but different implementations. Historically, AWX/Controller pivoted on attributes on the inventory source to figure out which ansible inventory to call. Now, instead of a single implementation we've split them into two implementations completely. This PR is to implement the change to pick which one we want and when.

Requires: ansible/awx-plugins#61

ISSUE TYPE
  • New or Enhanced Feature
COMPONENT NAME
  • Collection
AWX VERSION

ADDITIONAL INFORMATION

Copy link
Member

@webknjaz webknjaz left a comment

Choose a reason for hiding this comment

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

May I suggest a little refactoring?

awx/main/models/inventory.py Outdated Show resolved Hide resolved
awx/main/models/inventory.py Outdated Show resolved Hide resolved
@jessicamack jessicamack requested a review from webknjaz December 6, 2024 19:55
@github-actions github-actions bot added the dependencies Pull requests that update a dependency file label Dec 9, 2024
the related PR has been merged
@github-actions github-actions bot removed the dependencies Pull requests that update a dependency file label Dec 10, 2024
@chrismeyersfsu
Copy link
Member

The schema change failure here is a good thing. Prior to this PR we were in a state where we surfaced both the supported and unsupported version of the inventory plugins.

         "source": {
           "enum": [
             "azure_rm",
+            "controller",
             "ec2",
             "gce",
-            "vmware",
+            "insights",
+            "openshift_virtualization",
             "openstack",
             "rhv",
-            "rhv_supported",
             "satellite6",
-            "satellite6_supported",
             "terraform",
-            "controller",
-            "controller_supported",
-            "insights",
-            "insights_supported",
-            "openshift_virtualization",
-            "openshift_virtualization_supported",
+            "vmware",
             "scm",
             "constructed"
           ],
@@ -3336,21 +3331,16 @@
         "source": {
           "enum": [
             "azure_rm",
+            "controller",
             "ec2",
             "gce",
-            "vmware",
+            "insights",
+            "openshift_virtualization",
             "openstack",
             "rhv",
-            "rhv_supported",
make: *** [Makefile:548: detect-schema-change] Error 1
             "satellite6",
-            "satellite6_supported",
             "terraform",
-            "controller",
-            "controller_supported",
-            "insights",
-            "insights_supported",
-            "openshift_virtualization",
-            "openshift_virtualization_supported",
+            "vmware",
             "scm",
             "constructed"
           ],
@@ -3773,21 +3763,16 @@
         "source": {
           "enum": [
             "azure_rm",
+            "controller",
             "ec2",
             "gce",
-            "vmware",
+            "insights",
+            "openshift_virtualization",
             "openstack",
             "rhv",
-            "rhv_supported",
             "satellite6",
-            "satellite6_supported",
             "terraform",
-            "controller",
-            "controller_supported",
-            "insights",
-            "insights_supported",
-            "openshift_virtualization",
-            "openshift_virtualization_supported",
+            "vmware",
             "scm",
             "constructed"
           ],

@@ -1248,3 +1249,7 @@ def _new_func(*args, **kwargs):

def unified_job_class_to_event_table_name(job_class):
return f'main_{job_class().event_class.__name__.lower()}'


def load_all_entry_points_for(entry_point_subsections: list[str], /) -> dict[str, EntryPoint]:
Copy link
Member

Choose a reason for hiding this comment

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

Can you explain what the / is doing here? Maybe it's a new python thing I'm unaware of, but maybe it's unintended.

Copy link
Member Author

Choose a reason for hiding this comment

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

my understanding is that it's using this syntax https://docs.python.org/3.8/whatsnew/3.8.html#positional-only-parameters. it was suggested by @webknjaz here #15658 (comment). I think it's to enforce standard behavior.

@pb82
Copy link
Contributor

pb82 commented Dec 12, 2024

@jessicamack let's get this merged 👍

@jessicamack jessicamack merged commit c1f0a83 into ansible:devel Dec 16, 2024
25 of 26 checks passed
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.

6 participants