On Mon, May 21, 2012 at 05:23:02PM -0400, Matt Wagner wrote:
On Mon, May 21, 2012 at 02:52:13PM -0500, Steve Linabery wrote:
> This change to conductor is required to allow deletion of an
> existing aeolus user account. We need a rake task for it in the dc:
> namespace so that we can use ActiveRecord for the deletion.
>
> This is required so that we can easily create and delete a temporary
> user account with administrative privileges during aeolus-configure runs.
> ---
> src/lib/tasks/dc_tasks.rake | 21 +++++++++++++++++++++
> 1 files changed, 21 insertions(+), 0 deletions(-)
>
> diff --git a/src/lib/tasks/dc_tasks.rake b/src/lib/tasks/dc_tasks.rake
> index b83cc6d..f408f07 100644
> --- a/src/lib/tasks/dc_tasks.rake
> +++ b/src/lib/tasks/dc_tasks.rake
> @@ -30,6 +30,27 @@ namespace :dc do
> end
> end
>
> + desc 'Destroy an existing user'
> + task :destroy_user, [:login] => :environment do |t, args|
> + unless args.login
> + puts "Usage: rake 'dc:destroy_user[login]'"
> + exit(1)
> + end
> +
> + user = User.find_by_login(args.login)
> +
> + if !user
> + puts "User not found: #{args.login}"
> + exit(1)
> + end
> +
> + if user.destroy
Couldn't you collapse the above few lines into "elsif user.destroy"?
That doesn't really buy anything great, mind you. It's just how I would
have chosen to structure it.
> + puts "User #{args.login} destroyed"
> + else
> + puts "User destruction failed: #{args.login}"
Should we have an exit(1) here too, to indicate failure?
> + end
> + end
> +
>
> desc 'Create and register a list of ldap users, separated by
":"'
> task :create_ldap_users, [:logins] => :environment do |t, args|
> --
> 1.7.6.5
ACK, with two insignificant (i.e., disregard if you don't agree with
them) nits inline.
-- Matt
I like your points. I will incorporate them, test, and push it.
cheers,
s|e