Skip to content

Do not assume what is in os.environ#82495

Closed
malfet wants to merge 4 commits intomasterfrom
malfet-patch-1
Closed

Do not assume what is in os.environ#82495
malfet wants to merge 4 commits intomasterfrom
malfet-patch-1

Conversation

@malfet
Copy link
Copy Markdown
Contributor

@malfet malfet commented Jul 29, 2022

os.environ['FOO'] will raise IndexError if FOO is not found, while os.environ.get('FOO') would simply return None

Fixes #82492

`os.environ['FOO']` will raise `IndexError` if `FOO` is not found, while `os.environ.get('FOO')` would simply return `None`

Fixes #82492
@malfet malfet requested a review from a team as a code owner July 29, 2022 20:07
@pytorch-bot
Copy link
Copy Markdown

pytorch-bot bot commented Jul 29, 2022

🔗 Helpful Links

🧪 See artifacts and rendered test results here

Note: Links to docs will display an error until the docs builds have been completed.

❌ 1 Failures

As of commit 3a13b1e:

The following jobs have failed:

This comment was automatically generated by Dr. CI and updates every 15 minutes.

Copy link
Copy Markdown
Contributor

@huydhn huydhn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You might want to rebase this because @clee2000 puts a fix here last night #82452 (for a different issue part of the same issue)

@malfet
Copy link
Copy Markdown
Contributor Author

malfet commented Jul 29, 2022

You might want to rebase this because @clee2000 puts a fix here last night #82452 (for a different issue)

@huydhn well, it's a race then, as other PR has not landed yet :)

@malfet
Copy link
Copy Markdown
Contributor Author

malfet commented Jul 29, 2022

@pytorchbot merge -f "[OTHER] PR CI is green, no need to wait for trunk"

@pytorchmergebot
Copy link
Copy Markdown
Collaborator

@pytorchbot successfully started a merge job. Check the current status here

@github-actions
Copy link
Copy Markdown
Contributor

Hey @malfet.
You've committed this PR, but it does not have both a 'release notes: ...' and 'topics: ...' label. Please add one of each to the PR. The 'release notes: ...' label should represent the part of PyTorch that this PR changes (fx, autograd, distributed, etc) and the 'topics: ...' label should represent the kind of PR it is (not user facing, new feature, bug fix, perf improvement, etc). The list of valid labels can be found here for the 'release notes: ...' and here for the 'topics: ...'.
For changes that are 'topic: not user facing' there is no need for a release notes label.

facebook-github-bot pushed a commit that referenced this pull request Aug 1, 2022
Summary:
`os.environ['FOO']` will raise `IndexError` if `FOO` is not found, while `os.environ.get('FOO')` would simply return `None`

Fixes #82492

Pull Request resolved: #82495
Approved by: https://github.com/huydhn, https://github.com/kit1980

Test Plan: contbuild & OSS CI, see https://hud.pytorch.org/commit/pytorch/pytorch/b9cdd6d0ac66965466576da760cd4e2485911dd3

Reviewed By: osalpekar

Differential Revision: D38306853

Pulled By: malfet

fbshipit-source-id: 8bcd927c5efe12b0c9f084d7d32d6b733bd083b7
@github-actions github-actions bot deleted the malfet-patch-1 branch February 18, 2024 01:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to run pytorch unit test because CI environment variables are not available

5 participants