3
\$\begingroup\$

I created a load balancing server at work and have to keep track of which server it adds the user to, so I wrote a little Ruby script that will do just that.

I'm looking for any critique of my work, can I do anything better at all.

Source(user information, directory information, and server information changed for security):

#!/local/usr/bin/ruby

require 'net/ssh'

@username = 'user'
@password =  nil
@host     = 'server_to_run_command_on'

def loadbalance_server_data(user)
  check = Net::SSH.start(@host, @username, :password => @password)
  cmd = "finduser #{user}" #<= Custom command that finds user info
  res = check.exec!(cmd)
  data = res
  write_data(data)
end

def write_data(data)
  File.open("path/to/log.txt", "a+"){ |s| s.puts(data) }
end

user_list = %w(user user user).each do |user|
  loadbalance_server_data(user)
end

Example of log:

Querying: servers..

Server usage report for user
        server_name:  
        server_name:  
        server_name:  
        server_name:  
        server_name:  
        server_name:  
        server_name:  dbus-daemon gvfsd thunderbird #<= These programs always run on this server no matter what server they are logged in on
        server_name:  
        server_name:  
        server_name:  dbus-daemon dbus-launch do_4000_clone_u execrulz gam_server gnome-screensav gnome-settings- gnome-vfs-daemo newphp openbox openbox-session qccm soffice soffice.bin tint2 umessage #<= The server the user is logged in on
        server_name:  
\$\endgroup\$
  • \$\begingroup\$ I would also like to know if there is a way to have this program run at a certain time everyday? <- that is not on-topic here, the rest of the question is fine though. \$\endgroup\$ – Caridorc Dec 31 '15 at 15:25
  • \$\begingroup\$ See whats on topic here -> codereview.stackexchange.com/help/on-topic \$\endgroup\$ – Caridorc Dec 31 '15 at 15:31
  • 1
    \$\begingroup\$ @Caridorc Removed that chunk. \$\endgroup\$ – Bam Dec 31 '15 at 16:22
3
\$\begingroup\$

Constants in UPPERCASE

@username = 'user'
@password =  nil
@host     = 'server_to_run_command_on'

should be constants, as well as:

LOGFILE = "path/to/log.txt"

Marking a variable as constant in Ruby just needs upper-casing its name.

Return value simplification

res = check.exec!(cmd)
data = res
write_data(data)

Is unnecessarily long just write:

write_data( check.exec!(cmd) )

Other then this minor remarks, the code looks as straightforward and readable as it can get.

\$\endgroup\$
  • \$\begingroup\$ Why make them constants when I can just as easily make them instances? I understand the LOGFILE aspect of the constant, but not the hostname; username; pass part of it? \$\endgroup\$ – Bam Dec 31 '15 at 16:45
  • \$\begingroup\$ @Bam To me an instance outside of a class is confusing. Also a conatant will issue a warning if you try to modify it. \$\endgroup\$ – Caridorc Dec 31 '15 at 16:49
  • \$\begingroup\$ Or more clean:write_data check.exec! cmd \$\endgroup\$ – Nakilon Jan 1 '16 at 11:15

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.