pash: store user in a variable; r=bkero
authorGregory Szorc <gps@mozilla.com>
Thu, 05 Mar 2015 17:27:39 -0800
changeset 360505 779cf88d244327cf8dadbe15aa8b31f5281704b4
parent 360504 1fafbc32e07e46d5641eea23f6a91b4eac9b8674
child 360506 ec8d27a40f2d89444a86923acd6682dc2b0f3f85
push id16998
push userrwood@mozilla.com
push dateMon, 02 May 2016 19:42:03 +0000
reviewersbkero
pash: store user in a variable; r=bkero os.getenv('USER') was used multiple times in this file. The content of the environment variable doesn't change during execution. So get the value once and store it in a variable. Also, os.environ is the preferred mechanism to access environment variables from Python. os.getenv is a low-level API into the POSIX function of the same name. The difference doesn't matter in this use case, so use the method that is more accepted by the Python community.
scripts/pash/pash.py
--- a/scripts/pash/pash.py
+++ b/scripts/pash/pash.py
@@ -8,48 +8,51 @@ import sys
 
 import hg_helper
 import ldap_helper
 from sh_helper import QuoteForPOSIX
 
 
 if __name__ == '__main__':
     os.environ['PYTHONPATH'] = '/repo/hg/libraries/'
-    if os.getenv('USER') == 'root':
+
+    user = os.environ.get('USER')
+
+    if user == 'root':
         root_shell = pwd.getpwuid(0)[6]
-        ssh_command = os.getenv('SSH_ORIGINAL_COMMAND')
+        ssh_command = os.environ.get('SSH_ORIGINAL_COMMAND')
         if ssh_command:
             os.system(root_shell + " -c " + QuoteForPOSIX(ssh_command))
         else:
             os.execl(root_shell, root_shell)
     else:
-        server_port = os.getenv('SSH_CONNECTION').split ()[-1]
+        server_port = os.environ.get('SSH_CONNECTION').split ()[-1]
 
-        user_status = hg_helper.is_valid_user(os.getenv('USER'))
+        user_status = hg_helper.is_valid_user(user)
         if user_status == 2:
             sys.stderr.write('Your mercurial account has been disabled due \
                               to inactivity.\nPlease file a bug at \
                               https://bugzilla.mozilla.org (or \
                               http://tinyurl.com/2aveg9k) to re-activate \
                               your account.\n')
             sys.exit(0)
 
         elif user_status != 1:
             sys.stderr.write('You do not have a valid mercurial account!\n')
             sys.exit(0)
 
         # Run ldap access date toucher, silently fail and log if we're unable to write
         try:
-           ldap_helper.update_ldap_attribute(os.getenv('USER'), 'hgAccessDate',
+           ldap_helper.update_ldap_attribute(user, 'hgAccessDate',
                                              datetime.utcnow().strftime("%Y%m%d%H%M%S.%fZ"),
                                              'ldap://ldap.db.scl3.mozilla.com',
                                              'ldap://ldapsync1.db.scl3.mozilla.com')
         except Exception:
            logging.basicConfig(filename='/var/log/pash.log', level=logging.DEBUG)
-           logging.exception('Failed to update LDAP attributes for ' + os.getenv('USER'))
+           logging.exception('Failed to update LDAP attributes for %s' % user)
 
         # hg.mozilla.org handler
         if server_port == "22":
             hg_helper.serve('hg.mozilla.org')
 
         # hg.ecmascript.org handler
         elif server_port == "222":
             hg_helper.serve('hg.ecmascript.org')