search

aws-samples / amazon-sagemaker-notebook-instance-lifecycle-config-samples

A collection of sample scripts to customize Amazon SageMaker Notebook Instances using Lifecycle Configurations
MIT No Attribution
422 stars 250 forks source link

Revert "Auto stop print logging to catch idle state setting" #102

Closed sanmatti closed 1 year ago

sanmatti commented 1 year ago

Reverts aws-samples/amazon-sagemaker-notebook-instance-lifecycle-config-samples#95

These print statements break the auto-stop-idle LCC script when using SageMaker Notebook instance al1-v1 platform. Platform al1-v1 uses Python 2.7 to execute the autostop.py script, however f-strings formatting is supportd from Python 3.6. [1]

Specifically, using al1-v1 platform the script execution fails with following error:

  File "//autostop.py", line 101
    print(f'Notebook idle state set as {idle} because no kernel has been detected.')
                                                                                  ^
SyntaxError: invalid syntax

Please revert these changes as this can cause unwanted charges for users that rely on the auto-stop LCC to Stop idle NB instances that uses al1-v1 platform.

See open issue: https://github.com/aws-samples/amazon-sagemaker-notebook-instance-lifecycle-config-samples/issues/100

dmlause commented 1 year ago

Rather than reverting, can we update this PR to use a string formatting mechanism that will work in Python 2.7 and Python 3.6? i.e. %s operand? https://docs.python.org/3/tutorial/inputoutput.html#old-string-formatting

sanmatti commented 1 year ago

Sure, we can directly commit a change that replaces f-strings formatting with old-style %s operand.

Pete-rePete commented 1 year ago

we can directly commit a change that replaces f-strings formatting with old-style %s operand.

@sanmatti I believe this PR can be closed, since this recently merged PR does exactly what you described

sanmatti commented 1 year ago

Thank you @Pete-rePete, I'm closing this PR.