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

Add payload validity attributes to configuration_scripts #710

Merged
merged 2 commits into from
Nov 13, 2023

Conversation

agrare
Copy link
Member

@agrare agrare commented Nov 8, 2023

Add columns to store if the payload of a configuration_script is valid or not, and if not what the error is to help the user fix their scripts.

Add columns to store if the payload of a configuration_script is valid
or not, and if not what the error is to help the user fix their scripts.
@@ -0,0 +1,6 @@
class AddValidToConfigurationScripts < ActiveRecord::Migration[6.1]
def change
add_column :configuration_scripts, :payload_valid, :boolean
Copy link
Member

Choose a reason for hiding this comment

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

Should we set a default of true (or in Ruby use default_value_for)? Reason I'm asking is that ansible playbooks will be seen as "nil" valid, which is falsey in Ruby, so .payload_valid? will return false.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍 will do

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@miq-bot
Copy link
Member

miq-bot commented Nov 8, 2023

Checked commits agrare/manageiq-schema@def35ca~...bf65b56 with ruby 2.6.10, rubocop 1.28.2, haml-lint 0.35.0, and yamllint
1 file checked, 0 offenses detected
Everything looks fine. 👍

@Fryguy Fryguy merged commit 7a159b6 into ManageIQ:master Nov 13, 2023
2 checks passed
@agrare agrare deleted the add_configuration_script_valid branch November 13, 2023 15:38
@Fryguy
Copy link
Member

Fryguy commented Nov 14, 2023

Moving this to yes? until the parent PR is ready.

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.

3 participants