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

Refresh to pick up extra attributes of Ansible Credentials #14106

Merged
merged 1 commit into from
Mar 7, 2017

Conversation

jameswnl
Copy link
Contributor

We have added more attributes to Ansible credentials in #13826. This is to populate those.

@jameswnl
Copy link
Contributor Author

@miq-bot add_labels wip, enhancement, providers/ansible_tower

@jameswnl jameswnl closed this Mar 1, 2017
@jameswnl jameswnl reopened this Mar 1, 2017
@jameswnl jameswnl force-pushed the cred-attributes branch 5 times, most recently from 2dd8f12 to 5346db3 Compare March 1, 2017 21:55
@jameswnl jameswnl changed the title [WIP] Refresh to pick up extra attributes of Ansible Credentials Refresh to pick up extra attributes of Ansible Credentials Mar 1, 2017
@jameswnl
Copy link
Contributor Author

jameswnl commented Mar 1, 2017

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Mar 1, 2017
@jameswnl
Copy link
Contributor Author

jameswnl commented Mar 1, 2017

@durandom check this out. thanks

@@ -85,20 +85,25 @@ def assert_credentials
:name => "Demo Credential",
:userid => "admin",
)
expect(machine_credential.options.keys).to match_array(machine_credential.class::EXTRA_ATTRIBUTES.keys)
Copy link
Member

Choose a reason for hiding this comment

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

can you also test for the values here and in the other 2 tests?
Or are they empty?
If empty, maybe change to creds that are not empty?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I modified the machine credential and now added the check for a couple values. The other network/cloud credentials we don't have any extra attributes for them yet.

Copy link
Member

Choose a reason for hiding this comment

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

but its not yet in this PR?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😅 forgot the --force options when pushing. now done

@@ -99,6 +80,9 @@ def credentials
when 'openstack' then "#{provider_module}::AutomationManager::OpenstackCredential"
else "#{provider_module}::AutomationManager::Credential"
end
inventory_object.options = inventory_object.type.constantize::EXTRA_ATTRIBUTES.keys.each_with_object({}) do |k, h|
h[k] = credential.send(k)
Copy link
Member

Choose a reason for hiding this comment

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

prefer public_send

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done! thanks

@blomquisg
Copy link
Member

Close to kick tests

@blomquisg blomquisg closed this Mar 6, 2017
@blomquisg blomquisg reopened this Mar 6, 2017
@blomquisg
Copy link
Member

@durandom can you give me a green checkmark in the "Reviewers" section?

Copy link
Member

@durandom durandom left a comment

Choose a reason for hiding this comment

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

🚢 🇮🇹

@jameswnl jameswnl closed this Mar 7, 2017
@jameswnl jameswnl reopened this Mar 7, 2017
@jameswnl
Copy link
Contributor Author

jameswnl commented Mar 7, 2017

Rebased to pick up the new app/models/manager_refresh/inventory/core.rb and added options to it. Now travis should be happy.

@miq-bot
Copy link
Member

miq-bot commented Mar 7, 2017

Checked commit jameswnl@5557603 with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
5 files checked, 0 offenses detected
Everything looks good. 👍

@blomquisg blomquisg merged commit 659c7cf into ManageIQ:master Mar 7, 2017
@blomquisg blomquisg added this to the Sprint 56 Ending Mar 13, 2017 milestone Mar 7, 2017
@jameswnl jameswnl deleted the cred-attributes branch March 23, 2017 15:10
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.

5 participants