Skip to content

chore: remove dead Python 2 compatibility code in env_vars - #9152

Open
Andrej730 wants to merge 1 commit into
aws:developfrom
Andrej730:env-vars-cleanup
Open

chore: remove dead Python 2 compatibility code in env_vars#9152
Andrej730 wants to merge 1 commit into
aws:developfrom
Andrej730:env-vars-cleanup

Conversation

@Andrej730

@Andrej730 Andrej730 commented Aug 1, 2026

Copy link
Copy Markdown

Hello. Removing Python 2 compatibility code that was avoiding stringifying unicode strings, since Python 2 was deprecated long time ago and this code is now dead.

Mandatory Checklist

PRs will only be reviewed after checklist is complete

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@Andrej730
Andrej730 requested a review from a team as a code owner August 1, 2026 10:38
@github-actions github-actions Bot added area/local/start-api sam local start-api command area/local/invoke sam local invoke command area/local/start-invoke pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at. labels Aug 1, 2026
@roger-zhangg

Copy link
Copy Markdown
Member

Thanks for the cleanup, and apologies for the slow first response here.

I checked out the branch and verified the claim that this code is unreachable. It is — this is safe to merge as-is.

Minimum supported Python

pyproject.toml:10 declares requires-python = ">=3.10", and the classifiers block lists only 3.10 through 3.14. There is no supported interpreter below 3.10.

The removed branch cannot execute

On develop, samcli/local/lambdafn/env_vars.py:242 reads:

elif sys.version_info.major > Python.TWO:

Python.TWO is 2, so on every Python 3 interpreter this is 3 > 2 → always True, and control always lands on result = str(value) at :243. The two following branches at :244-247 are therefore dead:

elif not isinstance(value, unicode):  # noqa: F821 pylint: disable=undefined-variable
    result = str(value)
else:
    result = value

Worth noting that :244 references unicode, which is not a builtin on Python 3 at all — if that branch were ever reachable it would raise NameError, not return a value. The existing # noqa: F821 / pylint: disable=undefined-variable suppressions are themselves evidence the code was already known to be unresolvable. So the post-change else: result = str(value) is exactly behavior-preserving on all supported versions.

Removing the Python enum is not an API break

I grepped for every consumer of this module. The only symbol any other code imports from env_vars is EnvironmentVariables:

  • samcli/local/lambdafn/config.py:6
  • samcli/commands/local/lib/local_lambda.py:32
  • tests/unit/local/lambdafn/test_env_vars.py:8
  • tests/unit/local/lambdafn/test_runtime.py:11

Nothing references env_vars.Python, and after the change neither sys nor IntEnum is used anywhere else in the file, so dropping both imports is correct.

Test results

  • tests/unit/local/lambdafn/ (covers test_env_vars.py, test_runtime.py, test_config.py) — 131 passed, 0 failed.
  • ruff check and ruff format --check on the changed file — clean.
  • mypy on the changed file — no error attributable to this change. (It surfaces one pre-existing list-item error in the unrelated samcli/lib/utils/graphql_api.py:27, present on develop and untouched by this PR.)

On the merge state

For the record, GitHub reports mergeStateStatus: BLOCKED on this PR, but mergeable: MERGEABLE and reviewDecision: REVIEW_REQUIRED — so this is purely awaiting maintainer approval, not a rebase. Nothing for you to do.

No changes requested from me; this just needs a maintainer approval to move.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/local/invoke sam local invoke command area/local/start-api sam local start-api command area/local/start-invoke pr/external stage/needs-triage Automatically applied to new issues and PRs, indicating they haven't been looked at.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants