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

Change lookup to query to ensure list #647

Merged
merged 5 commits into from
Jul 17, 2023

Conversation

Rickmarges
Copy link
Contributor

@Rickmarges Rickmarges commented Jul 13, 2023

What does this PR do?

When first configuring an organization with only 1 inventory defined, the configuration fails with the message:
argument 'controller_inventories' is of type <class 'dict'> and we were unable to convert to list

The diff_role tasks use the lookup plugin to calculate the differences, which doesn't necessarily create a list when it recieves one item. This change will move from lookup to query, which will always return a list (see relevant link for more info)

How should this be tested?

  1. Create an empty organization (without inventories)
  2. Define 1 inventory for created organization
  3. Run the configuraion with diff_role and with_present: true

Other Relevant info, PRs, etc

https://docs.ansible.com/ansible/latest/plugins/lookup.html#forcing-lookups-to-return-lists-query-and-wantlist-true

@Rickmarges
Copy link
Contributor Author

We're still testing this further. We ran into some issues, but are not yet sure if they're related to this.
I will post an update ASAP

@Rickmarges
Copy link
Contributor Author

So, I've investigated the issue a bit more. Due to the last return statement (return [difference]) in combination with the query plugin from Ansible, caused certain lists to be set to a nested loop ([[...]]) instead of a single one ([...]).

With the currently added 2 commits, this issue should be resolved. And in my opinion this is ready to be merged once approved :)

@Rickmarges Rickmarges force-pushed the patch/change_to_query branch from 5ead506 to fe391ef Compare July 14, 2023 06:30
@Rickmarges Rickmarges force-pushed the patch/change_to_query branch from fe391ef to b01ea27 Compare July 14, 2023 06:48
@ivarmu ivarmu added bug Something isn't working good first issue Good for newcomers new New issue, this should be removed once reviewed labels Jul 14, 2023
Copy link
Contributor

@ivarmu ivarmu left a comment

Choose a reason for hiding this comment

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

LGTM

@ivarmu ivarmu requested a review from sean-m-sullivan July 14, 2023 07:06
@ivarmu ivarmu removed the good first issue Good for newcomers label Jul 14, 2023
Copy link
Collaborator

@sean-m-sullivan sean-m-sullivan left a comment

Choose a reason for hiding this comment

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

@ivarmu this looks good, looks like we need a changelog though as this is a breaking change, as well as the present/compare list bugfix,

Copy link
Collaborator

@sean-m-sullivan sean-m-sullivan left a comment

Choose a reason for hiding this comment

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

Changelog needed,

@sean-m-sullivan sean-m-sullivan self-requested a review July 17, 2023 15:27
@sean-m-sullivan sean-m-sullivan merged commit 80fb202 into redhat-cop:devel Jul 17, 2023
ivarmu added a commit to automationiberia/infra.aap_configuration that referenced this pull request Jul 31, 2023
sean-m-sullivan pushed a commit that referenced this pull request Aug 3, 2023
* added singulars to be treated as well

* new attributes for roles

* new attributes for roles

* added changelog fragment

* fix on map_item function

* removed extra empty line

* fixes on object_diff inputs

* removed debug information. added ORGANIZATIONLESS to credentials and users without an organization

* fix lintering issues

* fix lintering issues

* fix lintering issues

* tests fixes. multiple list from #647 fixed. Test ping URL fixed
djdanielsson pushed a commit to redhat-cop/aap_configuration_extended that referenced this pull request Oct 5, 2024
* added singulars to be treated as well

* new attributes for roles

* new attributes for roles

* added changelog fragment

* fix on map_item function

* removed extra empty line

* fixes on object_diff inputs

* removed debug information. added ORGANIZATIONLESS to credentials and users without an organization

* fix lintering issues

* fix lintering issues

* fix lintering issues

* tests fixes. multiple list from redhat-cop/infra.aap_configuration#647 fixed. Test ping URL fixed
djdanielsson pushed a commit to redhat-cop/aap_configuration_extended that referenced this pull request Oct 5, 2024
* added singulars to be treated as well

* new attributes for roles

* new attributes for roles

* added changelog fragment

* fix on map_item function

* removed extra empty line

* fixes on object_diff inputs

* removed debug information. added ORGANIZATIONLESS to credentials and users without an organization

* fix lintering issues

* fix lintering issues

* fix lintering issues

* tests fixes. multiple list from redhat-cop/infra.aap_configuration#647 fixed. Test ping URL fixed
przemkalit pushed a commit to przemkalit/aap_configuration that referenced this pull request Nov 22, 2024
* Change lookup to query to ensure list

* Prevent the diff plugin from returning a nested list

* Correctly make use of the present_list

* Add changelog fragment

---------

Co-authored-by: Marges, RSY (Rick) <[email protected]>
Co-authored-by: Ivan Aragonés Muniesa <[email protected]>
przemkalit pushed a commit to przemkalit/aap_configuration that referenced this pull request Nov 22, 2024
* Change lookup to query to ensure list

* Prevent the diff plugin from returning a nested list

* Correctly make use of the present_list

* Add changelog fragment

---------

Co-authored-by: Marges, RSY (Rick) <[email protected]>
Co-authored-by: Ivan Aragonés Muniesa <[email protected]>
przemkalit pushed a commit to przemkalit/aap_configuration that referenced this pull request Nov 22, 2024
* added singulars to be treated as well

* new attributes for roles

* new attributes for roles

* added changelog fragment

* fix on map_item function

* removed extra empty line

* fixes on object_diff inputs

* removed debug information. added ORGANIZATIONLESS to credentials and users without an organization

* fix lintering issues

* fix lintering issues

* fix lintering issues

* tests fixes. multiple list from redhat-cop#647 fixed. Test ping URL fixed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working filetree/object_diff new New issue, this should be removed once reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants