Skip to content

fix: replace bare except with except Exception (E722) - #5657

Closed
harshadkhetpal wants to merge 2 commits into
aws:masterfrom
harshadkhetpal:fix/bare-except-sagemaker-train
Closed

harshadkhetpal wants to merge 2 commits into
aws:masterfrom
harshadkhetpal:fix/bare-except-sagemaker-train

Conversation

@harshadkhetpal

Copy link
Copy Markdown

Summary

Replace bare except: clauses with explicit except Exception: in sagemaker-train utilities.

Why: Bare except: catches all exceptions including SystemExit, KeyboardInterrupt, and GeneratorExit (PEP 8 E722). Since these catch blocks are used for graceful fallbacks (not re-raising), except Exception: is the correct form — it avoids silently swallowing system-level signals.

Change:

# Before
except:
    pass

# After
except Exception:
    pass

Files Changed

  • sagemaker-train/src/sagemaker/train/evaluate/execution.py (2 instances)
  • sagemaker-train/src/sagemaker/train/common_utils/finetune_utils.py (1 instance)

Testing

No behavior change for normal exceptions — except Exception: catches all standard exceptions. The only difference is that KeyboardInterrupt, SystemExit, and GeneratorExit will now correctly propagate, which is the intended Python behavior.

@mohamedzeidan2021

Copy link
Copy Markdown
Collaborator

Hi, can you please resolve the conflicts if you're looking to get this merged ?

@mohamedzeidan2021

Copy link
Copy Markdown
Collaborator

Thanks for contributing! Seems like master already contains the except: Exception.

Closing PR. Please raise again if you have any concerns.

This branch is waiting to be deployed

1 waiting deployment
manual-approval — c9f9b4f1 Waiting Oct 5, 2026 by mohamedzeidan2021 via wait-for-approval #1635
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants