Log error message if os.rename fails

recipe_modules/bot_update/resources/bot_update.py

Index: recipe_modules/bot_update/resources/bot_update.py
diff --git a/recipe_modules/bot_update/resources/bot_update.py b/recipe_modules/bot_update/resources/bot_update.py
index 3211369667372e4ec45a6b52343120b69985a912..5300774934e4a2d722df208d7be90e9548d47818 100755
--- a/recipe_modules/bot_update/resources/bot_update.py
+++ b/recipe_modules/bot_update/resources/bot_update.py
@@ -299,7 +299,10 @@ def remove(target):
     os.makedirs(dead_folder)
   dest = path.join(dead_folder, uuid.uuid4().hex)
   print 'Marking for removal %s => %s' % (target, dest)
-  os.rename(target, dest)
+  try:
+    os.rename(target, dest)
+  except Exception as e:
+    print 'Error renaming %s to %s: %s' % (target, dest, str(e))

[ddoman1, 2016/10/25 21:33:08]

This would eat the exception and the execution flow would continue as if this
remove() call succeeded.

I believe that it would be more reasonable to interrupt the build with raising
the exception when an error occurred from os.rename() since os-level errors are
often critical and need someone to take a look at asap.

Also, I would be surprised if
#1. stderr from bot_update.py is not logged, and, therefore,
#2. bot_update.py needs to catch exceptions and print the message in stdout in
order for stderr messages to get logged.

https://build.chromium.org/p/chromium.win/builders/Win%20x64%20Builder/builds...

I expect that all stdout/stderr FDs should be connected to cloud tail or logdog
by remote_run (or at execution-level) such that each step doesn't have to stream
stderr to stdout by itself.
 
[stderr] link appears in step 'remote_run_result' only, and I haven't digged
into the recipe code to figure out how the stdout/stderr of each step are
logged. I would take a look at remote_run to find how it redirects the
stderr/stdout streams of the steps executed.

[agable, 2016/10/28 16:48:40]

You're absolutely right that we should be using the python logging library, and
reasonably sending things to stdout/stderr based on content. See the TODO at the
top of the file :)

However, for now, this `print` is in-line with the rest of the output in this
file. We should file a bug to convert the whole file over, but I wouldn't block
this CL on adopting best practices when it would be out-of-sync with the rest of
the file.

For what it's worth, all stdout and stderr from all recipe scripts is logged,
and is annotated according to stream (in buildbot logs, stdout is black while
stderr is red). So this will show up as black, which is unfortunate, but it will
show up and get logged and get streamed to logdog.

(But also yes, this should probably re-raise the exception too.)

[katthomas, 2016/10/28 18:41:13]

My bad on the raise. Done now.

 
 
 def ensure_no_checkout(dir_names):