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

[V2V] Conversion Host - Use JSON format for extra vars #18772

Conversation

ghost
Copy link

@ghost ghost commented May 16, 2019

In current implementation, the extra vars passed to the conversion host enablement playbook are separate strings. The problem with that is string escaping can be tricky, and it already creates problems with long multiline strings.

A solution is to use a single JSON string that contains all the extra vars. This PR implements that and the string is generated by .to_json which handles all the escaping for us.

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1710448

@ghost
Copy link
Author

ghost commented May 16, 2019

@miq-bot add-label transformation, bug, hammer/yes
@miq-bot add-reviewer @djberg96
@miq-bot add-reviewer @agrare
@miq-bot add-reviewer @jerryk55

@miq-bot
Copy link
Member

miq-bot commented May 16, 2019

Checked commit fabiendupont@d796ffd with ruby 2.3.3, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0
1 file checked, 0 offenses detected
Everything looks fine. 🍪

Copy link
Member

@agrare agrare left a comment

Choose a reason for hiding this comment

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

👍 LGTM nice change

@agrare agrare self-assigned this May 16, 2019
@agrare agrare merged commit ca1fe1f into ManageIQ:master May 16, 2019
@agrare agrare added this to the Sprint 112 Ending May 27, 2019 milestone May 16, 2019
@djberg96
Copy link
Contributor

👍 :)

@ghost ghost deleted the v2v_conversion_host_pass_extra_vars_as_json branch May 16, 2019 12:19
@ghost
Copy link
Author

ghost commented May 16, 2019

@djberg96 thanks for the idea.

@jerryk55
Copy link
Member

Not that it matters any more since its been merged, but simple enough and looks good!

simaishi pushed a commit that referenced this pull request May 20, 2019
…s_extra_vars_as_json

[V2V] Conversion Host - Use JSON format for extra vars

(cherry picked from commit ca1fe1f)

Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1711035
@simaishi
Copy link
Contributor

Hammer backport details:

$ git log -1
commit 6e61e20f38760fd3d30f5fe684a5164ddc0d5691
Author: Adam Grare <[email protected]>
Date:   Thu May 16 08:04:22 2019 -0400

    Merge pull request #18772 from fdupont-redhat/v2v_conversion_host_pass_extra_vars_as_json
    
    [V2V] Conversion Host - Use JSON format for extra vars
    
    (cherry picked from commit ca1fe1fa8515d7cd2b540c0b824ea475ae0de3ca)
    
    Fixes https://bugzilla.redhat.com/show_bug.cgi?id=1711035

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