{"action":"created","comment":{"url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/comments/137953333","pull_request_review_id":61688283,"id":137953333,"diff_hunk":"@@ -331,22 +345,28 @@ def ftp_url(self):\n     @property\n     def ftp(self):\n         if self._ftp is None:\n-            self._ftp = self._open_ftp()\n+            self._ftp = self._open_ftp(self.encoding)\n             self._welcome = self._ftp.getwelcome()\n         return self._ftp\n \n     @property\n+    def encoding(self):","path":"fs/ftpfs.py","position":158,"original_position":158,"commit_id":"8d3b88f6a16b83e203ef0fcceb96db9d8b5f0b11","original_commit_id":"8d3b88f6a16b83e203ef0fcceb96db9d8b5f0b11","user":{"login":"althonos","id":8660647,"avatar_url":"https://avatars3.githubusercontent.com/u/8660647?v=4","gravatar_id":"","url":"https://api.github.com/users/althonos","html_url":"https://github.com/althonos","followers_url":"https://api.github.com/users/althonos/followers","following_url":"https://api.github.com/users/althonos/following{/other_user}","gists_url":"https://api.github.com/users/althonos/gists{/gist_id}","starred_url":"https://api.github.com/users/althonos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/althonos/subscriptions","organizations_url":"https://api.github.com/users/althonos/orgs","repos_url":"https://api.github.com/users/althonos/repos","events_url":"https://api.github.com/users/althonos/events{/privacy}","received_events_url":"https://api.github.com/users/althonos/received_events","type":"User","site_admin":false},"body":"I agree with the first point, I'll change that.\r\n\r\nFor the second point: you have to set the encoding prior to connecting to the server. If you connect to the server with the default (latin-1), then change to utf-8 after seing the `UTF8` feature, the server will still think you're sending latin-1 encoded strings. So you to reset the server and use the proper encoding, you have to reconnect to the server with the new encoding.","created_at":"2017-09-10T16:09:09Z","updated_at":"2017-09-10T16:09:10Z","html_url":"https://github.com/PyFilesystem/pyfilesystem2/pull/68#discussion_r137953333","pull_request_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/68","author_association":"CONTRIBUTOR","_links":{"self":{"href":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/comments/137953333"},"html":{"href":"https://github.com/PyFilesystem/pyfilesystem2/pull/68#discussion_r137953333"},"pull_request":{"href":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/68"}},"in_reply_to_id":137926228},"pull_request":{"url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/68","id":138759371,"html_url":"https://github.com/PyFilesystem/pyfilesystem2/pull/68","diff_url":"https://github.com/PyFilesystem/pyfilesystem2/pull/68.diff","patch_url":"https://github.com/PyFilesystem/pyfilesystem2/pull/68.patch","issue_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/issues/68","number":68,"state":"open","locked":false,"title":"Update tests, and fix discovered bugs","user":{"login":"althonos","id":8660647,"avatar_url":"https://avatars3.githubusercontent.com/u/8660647?v=4","gravatar_id":"","url":"https://api.github.com/users/althonos","html_url":"https://github.com/althonos","followers_url":"https://api.github.com/users/althonos/followers","following_url":"https://api.github.com/users/althonos/following{/other_user}","gists_url":"https://api.github.com/users/althonos/gists{/gist_id}","starred_url":"https://api.github.com/users/althonos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/althonos/subscriptions","organizations_url":"https://api.github.com/users/althonos/orgs","repos_url":"https://api.github.com/users/althonos/repos","events_url":"https://api.github.com/users/althonos/events{/privacy}","received_events_url":"https://api.github.com/users/althonos/received_events","type":"User","site_admin":false},"body":"Hi Will, I updated the tests in `fs.test` with some missing checks, and discovered some new bugs I will fix gradually in this PR.\r\n\r\n### New tests\r\n1. `FS.copy`, `FS.move`, `FS.copydir` and `FS.movedir` should raise `ResourceNotFound` when the source is nowhere to be found\r\n2. `FS.copy` and `FS.move` should probably raise `FileExpected` when the source is a directory (not listed in the documentation, but makes sense I guess ?...)\r\n3. `FS.movedir` and `FS.copydir` should raise `DirectoryExpected` when the source is a file\r\n4. If a filesystem is advertising itself as *case insensitive*, check that's true\r\n5. If a filesystem is advertising itself as supporting *unicode paths*, check that's true\r\n6. Check that *file-like* objects returned by `FS.openbin`:\r\n    * have a `seek` method that returns the new absolute position when called\r\n    * have a `write` method that returns the number of bytes (in binary mode) / characters (in text mode) written\r\n    * have a `truncate` method that returns the new length of the file\r\n\r\n### Discovered bugs\r\n* ~`FTPFile.seek`, `FTPFile.truncate` and `FTPFile.write` do not return anything~ *fixed*\r\n* ~`FTPFS` crashes on unicode paths (`/földér`)~ *fixed, see below*\r\n* ~`MemoryFile.write` and `MemoryFile.truncate` do not return anything~ *fixed*\r\n* ~many filesystem's `move` and `copy` do not raise `FileExpected` when given a directory~ *fixed by fixing `FS.copy` and `FS.move`*\r\n\r\n### About FTPFS\r\nFTP servers supporting UTF-8 encoded paths display the feature `UTF8`. But the `ftplib.FTP` objects require to set the encoding *before* connecting to the server, or else the server and the client encoding will not be synchronised (client will send *utf-8* paths but server will treat them as *latin-1*). \r\n\r\nTo fix this, the encoding is checked when accessing the `FTPFS.ftp` property for the first time. A mock connection is established to check for the `UTF8` feature ; then, a proper connection is established, with *utf-8* as the encoding if available, and *latin-1* otherwise. Then, any new ftp connection will use the previous encoding, so the overhead is limited.\r\n\r\n\r\n### About test_ftpfs\r\nI rewrote the class so that a new server is created once by class instead of before each test, which was a lot longer for nothing. I also made use of `pyftpdlib.test.FTPd` instead of externally calling the file and then *task-killing* the thread, which was a lot hackier. \r\n\r\nI still think there is an issue with the timeout of `FTPFS` objects, since the `test_connection_error` tests take a really long time, longer than what they should take given the timeout.","created_at":"2017-08-31T20:30:57Z","updated_at":"2017-09-10T16:09:10Z","closed_at":null,"merged_at":null,"merge_commit_sha":"b4b22dc2b8b7910b06006584647b824f04b8c5e7","assignee":null,"assignees":[],"requested_reviewers":[],"milestone":null,"commits_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/68/commits","review_comments_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/68/comments","review_comment_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/comments{/number}","comments_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/issues/68/comments","statuses_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/statuses/8d3b88f6a16b83e203ef0fcceb96db9d8b5f0b11","head":{"label":"althonos:master","ref":"master","sha":"8d3b88f6a16b83e203ef0fcceb96db9d8b5f0b11","user":{"login":"althonos","id":8660647,"avatar_url":"https://avatars3.githubusercontent.com/u/8660647?v=4","gravatar_id":"","url":"https://api.github.com/users/althonos","html_url":"https://github.com/althonos","followers_url":"https://api.github.com/users/althonos/followers","following_url":"https://api.github.com/users/althonos/following{/other_user}","gists_url":"https://api.github.com/users/althonos/gists{/gist_id}","starred_url":"https://api.github.com/users/althonos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/althonos/subscriptions","organizations_url":"https://api.github.com/users/althonos/orgs","repos_url":"https://api.github.com/users/althonos/repos","events_url":"https://api.github.com/users/althonos/events{/privacy}","received_events_url":"https://api.github.com/users/althonos/received_events","type":"User","site_admin":false},"repo":{"id":102032280,"name":"pyfilesystem2","full_name":"althonos/pyfilesystem2","owner":{"login":"althonos","id":8660647,"avatar_url":"https://avatars3.githubusercontent.com/u/8660647?v=4","gravatar_id":"","url":"https://api.github.com/users/althonos","html_url":"https://github.com/althonos","followers_url":"https://api.github.com/users/althonos/followers","following_url":"https://api.github.com/users/althonos/following{/other_user}","gists_url":"https://api.github.com/users/althonos/gists{/gist_id}","starred_url":"https://api.github.com/users/althonos/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/althonos/subscriptions","organizations_url":"https://api.github.com/users/althonos/orgs","repos_url":"https://api.github.com/users/althonos/repos","events_url":"https://api.github.com/users/althonos/events{/privacy}","received_events_url":"https://api.github.com/users/althonos/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/althonos/pyfilesystem2","description":"Python's Filesystem abstraction layer","fork":true,"url":"https://api.github.com/repos/althonos/pyfilesystem2","forks_url":"https://api.github.com/repos/althonos/pyfilesystem2/forks","keys_url":"https://api.github.com/repos/althonos/pyfilesystem2/keys{/key_id}","collaborators_url":"https://api.github.com/repos/althonos/pyfilesystem2/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/althonos/pyfilesystem2/teams","hooks_url":"https://api.github.com/repos/althonos/pyfilesystem2/hooks","issue_events_url":"https://api.github.com/repos/althonos/pyfilesystem2/issues/events{/number}","events_url":"https://api.github.com/repos/althonos/pyfilesystem2/events","assignees_url":"https://api.github.com/repos/althonos/pyfilesystem2/assignees{/user}","branches_url":"https://api.github.com/repos/althonos/pyfilesystem2/branches{/branch}","tags_url":"https://api.github.com/repos/althonos/pyfilesystem2/tags","blobs_url":"https://api.github.com/repos/althonos/pyfilesystem2/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/althonos/pyfilesystem2/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/althonos/pyfilesystem2/git/refs{/sha}","trees_url":"https://api.github.com/repos/althonos/pyfilesystem2/git/trees{/sha}","statuses_url":"https://api.github.com/repos/althonos/pyfilesystem2/statuses/{sha}","languages_url":"https://api.github.com/repos/althonos/pyfilesystem2/languages","stargazers_url":"https://api.github.com/repos/althonos/pyfilesystem2/stargazers","contributors_url":"https://api.github.com/repos/althonos/pyfilesystem2/contributors","subscribers_url":"https://api.github.com/repos/althonos/pyfilesystem2/subscribers","subscription_url":"https://api.github.com/repos/althonos/pyfilesystem2/subscription","commits_url":"https://api.github.com/repos/althonos/pyfilesystem2/commits{/sha}","git_commits_url":"https://api.github.com/repos/althonos/pyfilesystem2/git/commits{/sha}","comments_url":"https://api.github.com/repos/althonos/pyfilesystem2/comments{/number}","issue_comment_url":"https://api.github.com/repos/althonos/pyfilesystem2/issues/comments{/number}","contents_url":"https://api.github.com/repos/althonos/pyfilesystem2/contents/{+path}","compare_url":"https://api.github.com/repos/althonos/pyfilesystem2/compare/{base}...{head}","merges_url":"https://api.github.com/repos/althonos/pyfilesystem2/merges","archive_url":"https://api.github.com/repos/althonos/pyfilesystem2/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/althonos/pyfilesystem2/downloads","issues_url":"https://api.github.com/repos/althonos/pyfilesystem2/issues{/number}","pulls_url":"https://api.github.com/repos/althonos/pyfilesystem2/pulls{/number}","milestones_url":"https://api.github.com/repos/althonos/pyfilesystem2/milestones{/number}","notifications_url":"https://api.github.com/repos/althonos/pyfilesystem2/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/althonos/pyfilesystem2/labels{/name}","releases_url":"https://api.github.com/repos/althonos/pyfilesystem2/releases{/id}","deployments_url":"https://api.github.com/repos/althonos/pyfilesystem2/deployments","created_at":"2017-08-31T18:30:58Z","updated_at":"2017-08-31T18:31:00Z","pushed_at":"2017-09-04T14:21:18Z","git_url":"git://github.com/althonos/pyfilesystem2.git","ssh_url":"git@github.com:althonos/pyfilesystem2.git","clone_url":"https://github.com/althonos/pyfilesystem2.git","svn_url":"https://github.com/althonos/pyfilesystem2","homepage":"https://www.pyfilesystem.org","size":657,"stargazers_count":0,"watchers_count":0,"language":"Python","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":0,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"PyFilesystem:master","ref":"master","sha":"f49c33afc0f8c1beb4219cd00f363dd939c50fcd","user":{"login":"PyFilesystem","id":11898830,"avatar_url":"https://avatars0.githubusercontent.com/u/11898830?v=4","gravatar_id":"","url":"https://api.github.com/users/PyFilesystem","html_url":"https://github.com/PyFilesystem","followers_url":"https://api.github.com/users/PyFilesystem/followers","following_url":"https://api.github.com/users/PyFilesystem/following{/other_user}","gists_url":"https://api.github.com/users/PyFilesystem/gists{/gist_id}","starred_url":"https://api.github.com/users/PyFilesystem/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/PyFilesystem/subscriptions","organizations_url":"https://api.github.com/users/PyFilesystem/orgs","repos_url":"https://api.github.com/users/PyFilesystem/repos","events_url":"https://api.github.com/users/PyFilesystem/events{/privacy}","received_events_url":"https://api.github.com/users/PyFilesystem/received_events","type":"Organization","site_admin":false},"repo":{"id":70920962,"name":"pyfilesystem2","full_name":"PyFilesystem/pyfilesystem2","owner":{"login":"PyFilesystem","id":11898830,"avatar_url":"https://avatars0.githubusercontent.com/u/11898830?v=4","gravatar_id":"","url":"https://api.github.com/users/PyFilesystem","html_url":"https://github.com/PyFilesystem","followers_url":"https://api.github.com/users/PyFilesystem/followers","following_url":"https://api.github.com/users/PyFilesystem/following{/other_user}","gists_url":"https://api.github.com/users/PyFilesystem/gists{/gist_id}","starred_url":"https://api.github.com/users/PyFilesystem/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/PyFilesystem/subscriptions","organizations_url":"https://api.github.com/users/PyFilesystem/orgs","repos_url":"https://api.github.com/users/PyFilesystem/repos","events_url":"https://api.github.com/users/PyFilesystem/events{/privacy}","received_events_url":"https://api.github.com/users/PyFilesystem/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/PyFilesystem/pyfilesystem2","description":"Python's Filesystem abstraction layer","fork":false,"url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2","forks_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/forks","keys_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/keys{/key_id}","collaborators_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/teams","hooks_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/hooks","issue_events_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/issues/events{/number}","events_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/events","assignees_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/assignees{/user}","branches_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/branches{/branch}","tags_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/tags","blobs_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/git/refs{/sha}","trees_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/git/trees{/sha}","statuses_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/statuses/{sha}","languages_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/languages","stargazers_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/stargazers","contributors_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/contributors","subscribers_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/subscribers","subscription_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/subscription","commits_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/commits{/sha}","git_commits_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/git/commits{/sha}","comments_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/comments{/number}","issue_comment_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/issues/comments{/number}","contents_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/contents/{+path}","compare_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/compare/{base}...{head}","merges_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/merges","archive_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/downloads","issues_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/issues{/number}","pulls_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls{/number}","milestones_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/milestones{/number}","notifications_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/labels{/name}","releases_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/releases{/id}","deployments_url":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/deployments","created_at":"2016-10-14T15:05:27Z","updated_at":"2017-09-08T14:38:28Z","pushed_at":"2017-09-09T18:08:25Z","git_url":"git://github.com/PyFilesystem/pyfilesystem2.git","ssh_url":"git@github.com:PyFilesystem/pyfilesystem2.git","clone_url":"https://github.com/PyFilesystem/pyfilesystem2.git","svn_url":"https://github.com/PyFilesystem/pyfilesystem2","homepage":"https://www.pyfilesystem.org","size":593,"stargazers_count":422,"watchers_count":422,"language":"Python","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":35,"mirror_url":null,"open_issues_count":10,"forks":35,"open_issues":10,"watchers":422,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/68"},"html":{"href":"https://github.com/PyFilesystem/pyfilesystem2/pull/68"},"issue":{"href":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/issues/68"},"comments":{"href":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/issues/68/comments"},"review_comments":{"href":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/68/comments"},"review_comment":{"href":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/pulls/68/commits"},"statuses":{"href":"https://api.github.com/repos/PyFilesystem/pyfilesystem2/statuses/8d3b88f6a16b83e203ef0fcceb96db9d8b5f0b11"}},"author_association":"CONTRIBUTOR"}}
{"actor":{"display_login":"althonos"}}