Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Error Handling in rewrite is incorrect. #413

Closed
theGOTOguy opened this issue Jul 23, 2022 · 3 comments
Closed

Error Handling in rewrite is incorrect. #413

theGOTOguy opened this issue Jul 23, 2022 · 3 comments

Comments

@theGOTOguy
Copy link
Contributor

Look at:
https://github.com/theskumar/python-dotenv/blob/master/src/dotenv/main.py#L136

If lines 136, 137 or 140 there are ever hit, an error "local variable 'dest' referenced before assignment" will be thrown, because the dest variable only exists within the scope of the with block above.
theGOTOguy added a commit to theGOTOguy/python-dotenv that referenced this issue Jul 27, 2022
@theGOTOguy
Fixes theskumar#413, whereby the NamedTemporaryFile "dest" was out of…
532a679 
… scope in the error handling portion of rewrite.
theskumar pushed a commit that referenced this issue Sep 3, 2022
@theGOTOguy
fix: out of scope error when "dest" variable is undefined #413
a53d652
theskumar added a commit that referenced this issue Sep 3, 2022
@theskumar
Revert "fix: out of scope error when "dest" variable is undefined #413"
d9fc48e 
This reverts commit a53d652.
theskumar added a commit that referenced this issue Sep 3, 2022
@theskumar
Revert "fix: out of scope error when "dest" variable is undefined #413"
490b116 
This reverts commit a53d652.
@theGOTOguy
Copy link
Contributor Author

Resolved by #414

@theskumar
Copy link
Owner

I had to revert #414 because the test was failing. Do you want to take another stab at it?

@theskumar theskumar reopened this Sep 4, 2022
@theGOTOguy
Copy link
Contributor Author

Looks like it was just a linter error. I submitted a new PR -- #423 should work.

@theGOTOguy theGOTOguy mentioned this issue Sep 4, 2022
Merged
bbc2 pushed a commit to theGOTOguy/python-dotenv that referenced this issue Nov 11, 2022
@theGOTOguy @bbc2
Fix out of scope error when "dest" variable is undefined
5f28ee6 
Fixes theskumar#413 whereby the NamedTemporaryFile "dest" was out of scope in the
error handling portion of rewrite.

The problem was initially fixed in theskumar#414 but it got reverted because of a
linter error. This new commit works around that linter error.
@bbc2 bbc2 closed this as completed in 1ecb57d Nov 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@theskumar @theGOTOguy