-
-
Notifications
You must be signed in to change notification settings - Fork 382
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
Ensure name attribute in ftp.py #843
Conversation
smart_open/ftp.py
Outdated
@@ -169,4 +169,5 @@ def full_close(self): | |||
fobj.socket = socket | |||
fobj.conn = conn | |||
fobj.close = types.MethodType(full_close, fobj) | |||
# fobj.name = path |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we don't need it, then let's remove it instead of commenting
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yep, the call was hanging because name
is a property of a socket file: https://github.com/python/cpython/blob/v3.11.1/Lib/socket.py#L758
so this PR is now adding tests and an assertion, meaning the tests are not able to reproduce the issue in question.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
from the issue:
unable to transparently decompress <_io.BufferedReader name=580> because it seems to lack a string-like .name
name=580, returned by fileno() in the link above, is not reproduced in integration tests (we get the .gz
apparently, otherwise the tests would fail)
can't set an attribute where the class has a property defined, so it's back to #842 |
Alternative to #842
Title
Please pick a concise, informative and complete title for your PR.
The title is important because it will appear in our change log.
Motivation
Please explain the motivation behind this PR in the description.
If you're fixing a bug, link to the issue number like so:
Fixes #841
If you're adding a new feature, then consider opening a ticket and discussing it with the maintainers before you actually do the hard work.
Tests
If you're fixing a bug, consider test-driven development:
If you're implementing a new feature, include unit tests for it.
Make sure all existing unit tests pass.
You can run them locally using:
If there are any failures, please fix them before creating the PR (or mark it as WIP, see below).
Work in progress
If you're still working on your PR, include "WIP" in the title.
We'll skip reviewing it for the time being.
Once you're ready to review, remove the "WIP" from the title, and ping one of the maintainers (e.g. mpenkov).
Checklist
Before you create the PR, please make sure you have:
Workflow
Please avoid rebasing and force-pushing to the branch of the PR once a review is in progress.
Rebasing can make your commits look a bit cleaner, but it also makes life more difficult from the reviewer, because they are no longer able to distinguish between code that has already been reviewed, and unreviewed code.