tvalentyn commented on a change in pull request #11067: [BEAM-9136]Add licenses 
for dependencies for Python
URL: https://github.com/apache/beam/pull/11067#discussion_r400642519
 
 

 ##########
 File path: sdks/python/container/license_scripts/pull_licenses_py.py
 ##########
 @@ -63,11 +61,13 @@ def copy_license_files(dep):
             dep=name))
     return True
   except Exception as e:
-    print(e)
-    return False
+    print(
+        'Failed to copy from {source} to {dest}'.format(
+            source=source_license_file, dest=dest_dir + '/LICENSE'))
+    traceback.print_exc()
 
 
-@retry(stop=stop_after_attempt(3))
+@retry(wait=wait_exponential(multiplier=1, min=30, max=180))
 
 Review comment:
   What would be retry the intervals with this spec?
   I think it's going to be max(2, 30) = 30, max(4, 30) = 30, ..., max(32, 30) 
= 32, then 64, 128, then 180, 180, 180, .. indefinitely.
   
   I would have an initial retry faster, and then increase the delay, and give 
up after a few attempts - perhaps somebody made a typo, so it's better to find 
out sooner than later. 
   
   WDYT about:
   `@retry(reraise=True, wait=wait_exponential(multiplier=2), 
stop=stop_after_attempt(5))`
   This should translate to :  4s, 8s, 16s, 32s -> timeout.
   `reraise` will capture the original stacktrace, instead of throwing 
`RetryError`.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to