Skip to content

bpo-42830: Update the documentation for tempfile.mkstemp #27075

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.

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

Closed
wants to merge 3 commits into from
Closed

bpo-42830: Update the documentation for tempfile.mkstemp #27075

wants to merge 3 commits into from

Conversation

srinivasreddy
Copy link
Contributor

@srinivasreddy srinivasreddy commented Jul 9, 2021

bpo-42830: Update the documentation for tempfile.mkstemp function that user is responsible for closing the file descriptor while closing the created file.

https://bugs.python.org/issue42830

…t user is responsible for closing the file descriptor while closing the created file.
Copy link
Contributor

@akulakov akulakov left a comment

Choose a reason for hiding this comment

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

Thanks! a couple of small comments.

Co-authored-by: andrei kulakov <andrei.avk@gmail.com>
@akulakov
Copy link
Contributor

akulakov commented Jul 9, 2021

Can you also update line 193 to replace 'handle' with 'file descriptor' (for consistency with os.close)?

Co-authored-by: andrei kulakov <andrei.avk@gmail.com>
@srinivasreddy
Copy link
Contributor Author

I do not know the difference between the two words in OS context. I am not sure.

@github-actions
Copy link

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Aug 10, 2021
@srinivasreddy srinivasreddy reopened this Nov 12, 2021
Comment on lines +165 to +167
for deleting the temporary file when done with it. If a large number of file
descriptors are created, your program may run into a per-process limit and raise an :exc:`OSError`.
You can avoid this issue by closing file descriptors with :func:`os.close`.
Copy link
Member

Choose a reason for hiding this comment

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

I would just say something like "the user of :func:mskstemp is responsible for deleting the temporary file and closing the file descriptor". Lots of things can cause you to run out of FDs, this isn't the place to describe the exact behavior.

Copy link
Member

Choose a reason for hiding this comment

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

I agree.

@fochoao
Copy link

fochoao commented Apr 21, 2022

Check this please... thanks for letting me be part of this project, and have done 8 contributions just that fact puts a smile in my face.

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Aug 10, 2022
@slateny
Copy link
Contributor

slateny commented Dec 6, 2022

@srinivasreddy Would you still interested in giving the PR a quick update per the review?

@srinivasreddy
Copy link
Contributor Author

Sure. I would do it in a day or two.

Copy link
Contributor

@hauntsaninja hauntsaninja left a comment

Choose a reason for hiding this comment

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

Requesting changes as per above :-)

@bedevere-bot
Copy link

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.