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

fixes for bugs detected during a backport #51

Open
wants to merge 8 commits into
base: devel
Choose a base branch
from

Conversation

ivarmu
Copy link
Contributor

@ivarmu ivarmu commented Dec 18, 2024

What does this PR do?

Fixes a bug detected during the backport of the latest enhacements. filetree_create job_template and workflow_job_template survey issue when the survey_spec was empty (but defined)

How should this be tested?

Manually tested.

Is there a relevant Issue open for this?

Other Relevant info, PRs, etc

Related to the backport PR redhat-cop/infra.controller_configuration#15

@ivarmu ivarmu requested review from silvinux, adonisgarciac and a team as code owners December 18, 2024 08:18
@ivarmu ivarmu requested a review from przemkalit January 8, 2025 09:58
@ivarmu ivarmu requested a review from przemkalit January 8, 2025 10:00
Copy link
Contributor

@przemkalit przemkalit left a comment

Choose a reason for hiding this comment

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

Thanks for restoring the change :).

@przemkalit
Copy link
Contributor

Hey, I found a bug related to the survey topic. The choices should be enclosed in double quotes; otherwise, the default directive for multi-choice type questions fails during the import, and as it is really minor change maybe you could add it here :-).

@ivarmu ivarmu requested a review from przemkalit January 13, 2025 07:45
@ivarmu
Copy link
Contributor Author

ivarmu commented Jan 13, 2025

Hey, I found a bug related to the survey topic. The choices should be enclosed in double quotes; otherwise, the default directive for multi-choice type questions fails during the import, and as it is really minor change maybe you could add it here :-).

I've checked that and I have no problems importing the generated output (I attach an example generated by filetree_create and imported successfully by filetree_read.

11_Demo Job Template.yaml.txt

@ivarmu ivarmu requested review from djdanielsson and przemkalit and removed request for przemkalit January 13, 2025 08:37
@przemkalit
Copy link
Contributor

Hey, I found a bug related to the survey topic. The choices should be enclosed in double quotes; otherwise, the default directive for multi-choice type questions fails during the import, and as it is really minor change maybe you could add it here :-).

I've checked that and I have no problems importing the generated output (I attach an example generated by filetree_create and imported successfully by filetree_read.

11_Demo Job Template.yaml.txt

Hey, did you try to use dispatch to add the object? Becuase it is failing then. Sorry for late response, I am on vacation.

@ivarmu
Copy link
Contributor Author

ivarmu commented Jan 16, 2025

Hey, I found a bug related to the survey topic. The choices should be enclosed in double quotes; otherwise, the default directive for multi-choice type questions fails during the import, and as it is really minor change maybe you could add it here :-).

I've checked that and I have no problems importing the generated output (I attach an example generated by filetree_create and imported successfully by filetree_read.

11_Demo Job Template.yaml.txt

Hey, did you try to use dispatch to add the object? Becuase it is failing then. Sorry for late response, I am on vacation.

Yesss... my tests include the filetree_read and the dispatch roles, and I also check that the target instance has been successfully configured.

Enjoy your vacations, we can continue the tests when returned 😇

@djdanielsson
Copy link
Contributor

@ivarmu is this ready to be merged?

@przemkalit
Copy link
Contributor

Okay @ivarmu maybe this works on AAP 2.5, but I have other case with survey, because Today I've noticed that there is an issue when string is not in quotes - one of our JT had : inside the question name.

So check my solution:

{%       if survey_item_content.key | regex_search('question_description|default') %}
{%         set current_content = survey_item_content.key + ': !unsafe "' + (survey_item_content.value | regex_replace("\n", "\\\\n") | regex_replace('"', '\\"')) + '"' %}
{%       elif survey_item_content.key is not match('choices') %}
{%         if survey_item_content.value is string %}
{%           set current_content = survey_item_content.key + ': "' + survey_item_content.value | string + '"' %}
{%         else %}
{%           set current_content = survey_item_content.key + ': ' + survey_item_content.value | string %}
{%         endif %}
{%       endif %}

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.

3 participants