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

Create/Update/Delete Ansible Tower Projects and Credentials via queue #14305

Merged
merged 5 commits into from
Mar 14, 2017

Conversation

jameswnl
Copy link
Contributor

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

@blomquisg
Copy link
Member

Granted this is early, but don't forget the update_in_provider_queue and delete_in_provider_queue methods.

task_ids = EmsRefresh.queue_refresh_task(manager)
task_ids.each { |tid| MiqTask.wait_for_taskid(tid) }

reload!
Copy link
Member

@blomquisg blomquisg Mar 13, 2017

Choose a reason for hiding this comment

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

I don't think this will work. There's no class level method reload!

> ManageIQ::Providers::AnsibleTower::AutomationManager::Credential.respond_to? :reload!
=> false

You could do something similar to the create_in_provider and return the updated inventory object.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is an instance method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do we want to make it as class method?

queue(manager.my_zone, "create_in_provider", [manager_id, params], "Creating #{name}")
end

private
Copy link
Member

Choose a reason for hiding this comment

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

Surprisingly, this won't set the following class methods as private.

Copy link
Member

Choose a reason for hiding this comment

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

You'll have to do private_class_method :method_name in order to get private class methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it does!

I've updated my code to use send, or else it won't work

Copy link
Member

Choose a reason for hiding this comment

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

Maybe ActiveSupport::Concern is smarter. In general adding private for class methods doesn't do anything for the class methods. It only affects the instance methods' visibility.

Copy link
Member

Choose a reason for hiding this comment

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

Well, I'll be:

> ManageIQ::Providers::AnsibleTower::AutomationManager::Credential.private_methods.select {|m| ["queue", "refresh"].include? (m.to_s) }
=> [:refresh, :queue]

:class_name => name,
:method_name => "create_in_provider",
:method_name => method_name,
Copy link
Member

Choose a reason for hiding this comment

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

This won't work for the instance methods. You'll have to pass in the id of the current instance in order to have it look up the instance to call the method.

Copy link
Member

Choose a reason for hiding this comment

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

See here:

# :instance_id (if using an instance method...an id)

Copy link
Contributor Author

@jameswnl jameswnl Mar 13, 2017

Choose a reason for hiding this comment

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

yes!!!
It was in my mind, and it slippppped...

end

def update_in_provider_queue(params)
self.class.send('queue', resource.my_zone, "update_in_provider", [params], "Updating #{self.class.name}")
Copy link
Member

Choose a reason for hiding this comment

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

Re: https://github.com/ManageIQ/manageiq/pull/14305/files#r105751307

For instance, here, you'd have to pass in the self.id as well as update_in_provider. Otherwise, it will try to call a class method update_in_provider, which doesn't exist.

end

def create_in_provider_queue(manager_id, params)
def queue(my_zone, method_name, args, action)
Copy link
Member

Choose a reason for hiding this comment

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

I'd call my_zone just zone to avoid confusion with the my_zone method that exists in some contexts.

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!

@jameswnl jameswnl force-pushed the api-UD-cred branch 2 times, most recently from d9af78c to c29d667 Compare March 13, 2017 21:48
@jameswnl jameswnl changed the title [WIP] To update/delete a Tower credential To update/delete a Tower credential Mar 13, 2017
@jameswnl
Copy link
Contributor Author

@miq-bot remove_label wip

@miq-bot miq-bot removed the wip label Mar 13, 2017
@jameswnl jameswnl changed the title To update/delete a Tower credential To update/delete a Tower credential/project Mar 13, 2017

def create_in_provider_queue(manager_id, params)
manager = ExtManagementSystem.find(manager_id)
queue(manager.my_zone, nil, "create_in_provider", [manager_id, params], "Creating #{name}")
Copy link
Member

Choose a reason for hiding this comment

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

The last argument will be logged when creating the Queue item. Can you change that to include the ems_ref for the project being created? Something like "Creating #{name} with name #{params['name']}" maybe...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@blomquisg name or manager_ref(the native Tower ID), or id (vmdb record ID), which one is better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So ended up using name here because this object is not created yet. The update/delete are using manager_ref now. Done!

end

def update_in_provider_queue(params)
self.class.send('queue', manager.my_zone, id, "update_in_provider", [params], "Updating #{self.class.name}")
Copy link
Member

Choose a reason for hiding this comment

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

Same basic comment. The "Updating ..." action should contain an ID for the thing being updated. If there's an error in updating, we'll want to know which one caused the problem.

end

def delete_in_provider_queue
self.class.send('queue', manager.my_zone, id, "delete_in_provider", [], "Deleting #{self.class.name}")
Copy link
Member

Choose a reason for hiding this comment

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


def create_in_provider_queue(manager_id, params)
manager = ExtManagementSystem.find(manager_id)
queue(manager.my_zone, nil, "create_in_provider", [manager_id, params], "Creating #{name}")
Copy link
Member

Choose a reason for hiding this comment

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

end

def update_in_provider_queue(params)
self.class.send('queue', resource.my_zone, id, "update_in_provider", [params], "Updating #{self.class.name}")
Copy link
Member

Choose a reason for hiding this comment

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

end

def delete_in_provider_queue
self.class.send('queue', resource.my_zone, id, "delete_in_provider", [], "Deleting #{self.class.name}")
Copy link
Member

Choose a reason for hiding this comment

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

@miq-bot
Copy link
Member

miq-bot commented Mar 14, 2017

Checked commits jameswnl/manageiq@a9303cd~...144c8de with ruby 2.2.6, rubocop 0.47.1, and haml-lint 0.20.0
4 files checked, 0 offenses detected
Everything looks good. 🍰

@blomquisg blomquisg merged commit bb9c250 into ManageIQ:master Mar 14, 2017
@blomquisg blomquisg added this to the Sprint 57 Ending Mar 27, 2017 milestone Mar 14, 2017
@jameswnl jameswnl deleted the api-UD-cred branch March 16, 2017 19:09
@blomquisg blomquisg changed the title To update/delete a Tower credential/project Create/Update/Delete Ansible Tower Projects and Credentials via queue Mar 29, 2017
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