harvard / cloudJHub

An implementation of JupyterHub within the Amazon cloud, with automatic scaling up and down
BSD 3-Clause "New" or "Revised" License
126 stars 14 forks source link

Updating cull_idle_servers #12

Closed thiagoalmeidasa closed 6 years ago

thiagoalmeidasa commented 6 years ago

Fixing else indentation on retry function.

kyfantaz commented 6 years ago

@farassadek, are you sure this is an accurate change that should be accepted? Because of the return statement on L49, I don't believe the else clause will ever get called where it's been moved. I believe the original intent was that the else clause belonged to for loop for when retry fails after max_retries attempts.

I am guessing that the intent of @thiagoalmeidasa of moving the else clause to thetry except block is to have a logging statement per retry failure; however, this change as is will not accomplish that due to the previously mentioned return statement.

farassadek commented 6 years ago

@kyfantaz the execution of the else statement make an error because of the "raise e" that have a non defined variable "e". "e" is defined in the except statement. I accept the pull temporarily because of the error of the undefined variable "e". I will fix this by putting the else back to the "for" statement but I will remove the "raise e".

kyfantaz commented 6 years ago

👍