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

Update required variables for controller_notification_templates #1004

Closed
wants to merge 1 commit into from

Conversation

MallocArray
Copy link
Contributor

Lines 7 and 8 of https://github.com/MallocArray/infra.aap_configuration/blob/devel/roles/controller_notification_templates/tasks/main.yml show this is a mandatory variable

What does this PR do?

Update organization and notificaiton_type variables to be shown as required. The current task sets them as mandatory and they are required in the AWX GUI as well

How should this be tested?

Is there a relevant Issue open for this?

resolves #[number]

Other Relevant info, PRs, etc

@MallocArray MallocArray requested a review from a team as a code owner December 11, 2024 16:30
Copy link
Collaborator

@djdanielsson djdanielsson left a comment

Choose a reason for hiding this comment

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

This will need some further review, when looking at the module documentation, it does not show those as required and thus the code itself might need to be updated. Need to verify what is actually correct.

@MallocArray
Copy link
Contributor Author

MallocArray commented Dec 11, 2024

I had noticed that as well, since
https://docs.ansible.com/ansible/latest/collections/awx/awx/notification_template_module.html
doesn't show it as Required, but in the AWX interface, it clearly is
image

Perhaps it isn't required if state: absent is set, so the documentation doesn't show it as required

@djdanielsson
Copy link
Collaborator

I think that might be the case and thus idk if in code we want to make it required because some people might want to try and remove stuff. I am fine with updating the docs to say required but I think in the code we should remove that mandatory so remove can work.

@MallocArray
Copy link
Contributor Author

I agree, I think it would be better to remove the Mandatory filter on them and leave the documentation as not required, which will result in matching up with the official documentation for the module and make it the most flexible.

@djdanielsson
Copy link
Collaborator

Feel free to make those changes if you want or I can get to it soon

@MallocArray
Copy link
Contributor Author

Replacing with #1006

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants