Skip to content

Conversation

@kstrenkova
Copy link
Contributor

The pre_tempest and post_tempest variables are now deprecated and the pre_tests and post_tests variables should be used now. This patch removes all the instances of these old variables.

@kstrenkova kstrenkova requested a review from a team as a code owner October 16, 2025 07:39
@openshift-ci
Copy link
Contributor

openshift-ci bot commented Oct 16, 2025

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

evallesp
evallesp previously approved these changes Oct 16, 2025
update-edpm.yml Outdated
vars:
pre_tests: "{{ (lookup('vars', 'pre_tempest', default=[])) }}"
post_tests: "{{ (lookup('vars', 'post_tempest', default=[])) }}"
pre_tests: []
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this setting to an empty value to provide a default value because the value will be overridden? I'm always confused by the ansible precedence rules (https://docs.ansible.com/ansible/latest/playbook_guide/playbooks_variables.html#understanding-variable-precedence).

In other words, do we need to keep the empty definitions here (and in the other file)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggest to remove the vars and set default value in place where it is used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ack, I changed it to empty list just in case it was used somewhere 🤔

After going through the code I think the variables are not needed, if I understand correctly. It goes to the run_tests.yml playbook which calls the role run_hook and there is no hooks variable being set. There might be dependency even deeper, but to be honest I did not find it, so I just removed the variables.

The pre_tempest and post_tempest variables are now deprecated and
the pre_tests and post_tests variables should be used now. This
patch removes all the instances of these old variables.
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.

4 participants