{"action":"created","comment":{"url":"https://api.github.com/repos/yola/yodeploy/pulls/comments/81556577","pull_request_review_id":2527753,"id":81556577,"diff_hunk":"@@ -105,15 +105,15 @@ def hook(self, hook, target, repository, version):\n         if target:\n             cmd += ['--target', target]\n         cmd += [self.appdir, version]\n-        try:\n-            subprocess.check_call(cmd, env={\n-                    'PATH': os.environ['PATH'],\n-            })\n-        except subprocess.CalledProcessError:\n+\n+        cmd_proc = subprocess.Popen(\n+            cmd, env={'PATH': os.environ['PATH']}, close_fds=False)\n+        cmd_proc.wait()\n+        tlss.shutdown()","path":"yodeploy/application.py","position":null,"original_position":13,"commit_id":"dc3d4d6a1d61237dbef528b33319ff75683c02da","original_commit_id":"8db6e222118edcb50c75f3529094d3590876ac51","user":{"login":"jamesodo","id":435365,"avatar_url":"https://avatars.githubusercontent.com/u/435365?v=3","gravatar_id":"","url":"https://api.github.com/users/jamesodo","html_url":"https://github.com/jamesodo","followers_url":"https://api.github.com/users/jamesodo/followers","following_url":"https://api.github.com/users/jamesodo/following{/other_user}","gists_url":"https://api.github.com/users/jamesodo/gists{/gist_id}","starred_url":"https://api.github.com/users/jamesodo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jamesodo/subscriptions","organizations_url":"https://api.github.com/users/jamesodo/orgs","repos_url":"https://api.github.com/users/jamesodo/repos","events_url":"https://api.github.com/users/jamesodo/events{/privacy}","received_events_url":"https://api.github.com/users/jamesodo/received_events","type":"User","site_admin":false},"body":"Maybe this should be in a try/except/finally block in case `Popen` raises an `OSError` but `cmd` looks like something we know for sure exists so maybe not...","created_at":"2016-10-03T14:05:43Z","updated_at":"2016-10-03T14:05:43Z","html_url":"https://github.com/yola/yodeploy/pull/177#discussion_r81556577","pull_request_url":"https://api.github.com/repos/yola/yodeploy/pulls/177","_links":{"self":{"href":"https://api.github.com/repos/yola/yodeploy/pulls/comments/81556577"},"html":{"href":"https://github.com/yola/yodeploy/pull/177#discussion_r81556577"},"pull_request":{"href":"https://api.github.com/repos/yola/yodeploy/pulls/177"}}},"pull_request":{"url":"https://api.github.com/repos/yola/yodeploy/pulls/177","id":87659757,"html_url":"https://github.com/yola/yodeploy/pull/177","diff_url":"https://github.com/yola/yodeploy/pull/177.diff","patch_url":"https://github.com/yola/yodeploy/pull/177.patch","issue_url":"https://api.github.com/repos/yola/yodeploy/issues/177","number":177,"state":"open","locked":false,"title":"Experimental Python 3 support","user":{"login":"thomasw","id":154292,"avatar_url":"https://avatars.githubusercontent.com/u/154292?v=3","gravatar_id":"","url":"https://api.github.com/users/thomasw","html_url":"https://github.com/thomasw","followers_url":"https://api.github.com/users/thomasw/followers","following_url":"https://api.github.com/users/thomasw/following{/other_user}","gists_url":"https://api.github.com/users/thomasw/gists{/gist_id}","starred_url":"https://api.github.com/users/thomasw/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/thomasw/subscriptions","organizations_url":"https://api.github.com/users/thomasw/orgs","repos_url":"https://api.github.com/users/thomasw/repos","events_url":"https://api.github.com/users/thomasw/events{/privacy}","received_events_url":"https://api.github.com/users/thomasw/received_events","type":"User","site_admin":false},"body":"This PR should resolve most of the python 3 incompatibilities in yodeploy in a 2.7 backwards compatible way. I've tested it on https://stefanomorelikestefayes.env.yola.net and things seem to be working as they did before (there are some build/deploy failures but they appear to be unrelated to these changes).\r\n\r\n**This hasn't yet been used to deploy real apps in a real python 3 environment.** Since various bits of yodeploy involve downloading published versions of yodeploy from a package index, it's easiest to take that next step after first publishing a yodeploy version with the experimental support added here. It's likely that additional issues will be uncovered once we have an experimental python 3 environment to test this in.\r\n\r\nWith the above in mind, the bulk of the work here focuses on two main points:\r\n\r\n1. Ensuring that the existing test suite passes under python 3.4+.\r\n2. Ensuring that the changes made to achieve point 1 are done so in a way that is backwards compatible with Python 2.7.\r\n\r\nI highly recommend reviewing this commit by commit. I've taken care to include explanations for my changes in the commit messages.\r\n\r\n@stefanor, I'd appreciate it if you could dig into this when you have the time. There are a few issue in particular that I'd like your thoughts on:\r\n\r\n* test_application was installing the latest version of yodeploy from the configured target package index into a test virtualenv. That version isn't compatible with Python 3. To resolve that, I modified test_application to install the current checkout instead. Initially I did this just as a hack to make sure that things were working and I was going to revert that work and just skip those tests if `sys.version_info > (3, 0)` (with the aim of removing that skip once a py3 compatible version is published). I haven't done that yet because my hack seemed like a reasonable solution once I got to thinking about it. However, it comes with a big caveat at the moment: If you break your yodeploy checkout (because you're working on it or whatever) and it is still installable, you may end up with a cached test virtualenv that's broken in test-data. So, I could: 1. make the tests reinstall the current checkout every time you run the tests (tedious, but not impossible I don't think) 2. Always blow away test-data (+ ~1 minute to test runs). 3. Skip those tests under python 3 until this stuff gets released. \r\n* yodeploy doesn't include python version information in its requirements.txt hashes. I'm assuming this will be problematic for things like yodeploy in particular where we'd want to test/deploy in multiple Python envs. We might end up downloading a python 2 virtualenv when we want a python 3 env and vice versa? I'd considered modifying https://github.com/yola/yodeploy/blob/0c0f9eddd268ba2fc43c24f982bbcfacd784fe24/yodeploy/virtualenv.py#L33 to include the version string IFF we are using python 3 (so that our legacy python 2 requirements.txt hashes are still valid).\r\n\r\nIf any of you check this out locally to play around with it, keep these issues in mind:\r\n\r\n1. There is a race condition in LockFile that results in intermittent test failures. https://github.com/yola/yodeploy/issues/176\r\n2. The test suite writes to the checkout directory and, as a result, you can't (easily) run the test suite in multiple environments simultaneously. This is probably something we should address at some point. This pull includes some changes to avoid collisions in the subset of test data that gets cached between test runs but it doesn't resolve this problem globally.\r\n\r\nTake your time on reviewing this. I'm out of hours for now, but I'll respond as soon as I can.","created_at":"2016-10-03T03:34:39Z","updated_at":"2016-10-03T14:05:43Z","closed_at":null,"merged_at":null,"merge_commit_sha":"f5609657aaa3511c00c3e1c67cc38b06a450115d","assignee":null,"assignees":[],"milestone":null,"commits_url":"https://api.github.com/repos/yola/yodeploy/pulls/177/commits","review_comments_url":"https://api.github.com/repos/yola/yodeploy/pulls/177/comments","review_comment_url":"https://api.github.com/repos/yola/yodeploy/pulls/comments{/number}","comments_url":"https://api.github.com/repos/yola/yodeploy/issues/177/comments","statuses_url":"https://api.github.com/repos/yola/yodeploy/statuses/dc3d4d6a1d61237dbef528b33319ff75683c02da","head":{"label":"yola:3.5-compat","ref":"3.5-compat","sha":"dc3d4d6a1d61237dbef528b33319ff75683c02da","user":{"login":"yola","id":861497,"avatar_url":"https://avatars.githubusercontent.com/u/861497?v=3","gravatar_id":"","url":"https://api.github.com/users/yola","html_url":"https://github.com/yola","followers_url":"https://api.github.com/users/yola/followers","following_url":"https://api.github.com/users/yola/following{/other_user}","gists_url":"https://api.github.com/users/yola/gists{/gist_id}","starred_url":"https://api.github.com/users/yola/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yola/subscriptions","organizations_url":"https://api.github.com/users/yola/orgs","repos_url":"https://api.github.com/users/yola/repos","events_url":"https://api.github.com/users/yola/events{/privacy}","received_events_url":"https://api.github.com/users/yola/received_events","type":"Organization","site_admin":false},"repo":{"id":4809050,"name":"yodeploy","full_name":"yola/yodeploy","owner":{"login":"yola","id":861497,"avatar_url":"https://avatars.githubusercontent.com/u/861497?v=3","gravatar_id":"","url":"https://api.github.com/users/yola","html_url":"https://github.com/yola","followers_url":"https://api.github.com/users/yola/followers","following_url":"https://api.github.com/users/yola/following{/other_user}","gists_url":"https://api.github.com/users/yola/gists{/gist_id}","starred_url":"https://api.github.com/users/yola/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yola/subscriptions","organizations_url":"https://api.github.com/users/yola/orgs","repos_url":"https://api.github.com/users/yola/repos","events_url":"https://api.github.com/users/yola/events{/privacy}","received_events_url":"https://api.github.com/users/yola/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/yola/yodeploy","description":"Python library for deployment","fork":false,"url":"https://api.github.com/repos/yola/yodeploy","forks_url":"https://api.github.com/repos/yola/yodeploy/forks","keys_url":"https://api.github.com/repos/yola/yodeploy/keys{/key_id}","collaborators_url":"https://api.github.com/repos/yola/yodeploy/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/yola/yodeploy/teams","hooks_url":"https://api.github.com/repos/yola/yodeploy/hooks","issue_events_url":"https://api.github.com/repos/yola/yodeploy/issues/events{/number}","events_url":"https://api.github.com/repos/yola/yodeploy/events","assignees_url":"https://api.github.com/repos/yola/yodeploy/assignees{/user}","branches_url":"https://api.github.com/repos/yola/yodeploy/branches{/branch}","tags_url":"https://api.github.com/repos/yola/yodeploy/tags","blobs_url":"https://api.github.com/repos/yola/yodeploy/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/yola/yodeploy/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/yola/yodeploy/git/refs{/sha}","trees_url":"https://api.github.com/repos/yola/yodeploy/git/trees{/sha}","statuses_url":"https://api.github.com/repos/yola/yodeploy/statuses/{sha}","languages_url":"https://api.github.com/repos/yola/yodeploy/languages","stargazers_url":"https://api.github.com/repos/yola/yodeploy/stargazers","contributors_url":"https://api.github.com/repos/yola/yodeploy/contributors","subscribers_url":"https://api.github.com/repos/yola/yodeploy/subscribers","subscription_url":"https://api.github.com/repos/yola/yodeploy/subscription","commits_url":"https://api.github.com/repos/yola/yodeploy/commits{/sha}","git_commits_url":"https://api.github.com/repos/yola/yodeploy/git/commits{/sha}","comments_url":"https://api.github.com/repos/yola/yodeploy/comments{/number}","issue_comment_url":"https://api.github.com/repos/yola/yodeploy/issues/comments{/number}","contents_url":"https://api.github.com/repos/yola/yodeploy/contents/{+path}","compare_url":"https://api.github.com/repos/yola/yodeploy/compare/{base}...{head}","merges_url":"https://api.github.com/repos/yola/yodeploy/merges","archive_url":"https://api.github.com/repos/yola/yodeploy/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/yola/yodeploy/downloads","issues_url":"https://api.github.com/repos/yola/yodeploy/issues{/number}","pulls_url":"https://api.github.com/repos/yola/yodeploy/pulls{/number}","milestones_url":"https://api.github.com/repos/yola/yodeploy/milestones{/number}","notifications_url":"https://api.github.com/repos/yola/yodeploy/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/yola/yodeploy/labels{/name}","releases_url":"https://api.github.com/repos/yola/yodeploy/releases{/id}","deployments_url":"https://api.github.com/repos/yola/yodeploy/deployments","created_at":"2012-06-27T14:06:15Z","updated_at":"2016-05-26T16:32:48Z","pushed_at":"2016-10-03T11:15:04Z","git_url":"git://github.com/yola/yodeploy.git","ssh_url":"git@github.com:yola/yodeploy.git","clone_url":"https://github.com/yola/yodeploy.git","svn_url":"https://github.com/yola/yodeploy","homepage":null,"size":533,"stargazers_count":2,"watchers_count":2,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":17,"forks":0,"open_issues":17,"watchers":2,"default_branch":"master"}},"base":{"label":"yola:master","ref":"master","sha":"0c0f9eddd268ba2fc43c24f982bbcfacd784fe24","user":{"login":"yola","id":861497,"avatar_url":"https://avatars.githubusercontent.com/u/861497?v=3","gravatar_id":"","url":"https://api.github.com/users/yola","html_url":"https://github.com/yola","followers_url":"https://api.github.com/users/yola/followers","following_url":"https://api.github.com/users/yola/following{/other_user}","gists_url":"https://api.github.com/users/yola/gists{/gist_id}","starred_url":"https://api.github.com/users/yola/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yola/subscriptions","organizations_url":"https://api.github.com/users/yola/orgs","repos_url":"https://api.github.com/users/yola/repos","events_url":"https://api.github.com/users/yola/events{/privacy}","received_events_url":"https://api.github.com/users/yola/received_events","type":"Organization","site_admin":false},"repo":{"id":4809050,"name":"yodeploy","full_name":"yola/yodeploy","owner":{"login":"yola","id":861497,"avatar_url":"https://avatars.githubusercontent.com/u/861497?v=3","gravatar_id":"","url":"https://api.github.com/users/yola","html_url":"https://github.com/yola","followers_url":"https://api.github.com/users/yola/followers","following_url":"https://api.github.com/users/yola/following{/other_user}","gists_url":"https://api.github.com/users/yola/gists{/gist_id}","starred_url":"https://api.github.com/users/yola/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/yola/subscriptions","organizations_url":"https://api.github.com/users/yola/orgs","repos_url":"https://api.github.com/users/yola/repos","events_url":"https://api.github.com/users/yola/events{/privacy}","received_events_url":"https://api.github.com/users/yola/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/yola/yodeploy","description":"Python library for deployment","fork":false,"url":"https://api.github.com/repos/yola/yodeploy","forks_url":"https://api.github.com/repos/yola/yodeploy/forks","keys_url":"https://api.github.com/repos/yola/yodeploy/keys{/key_id}","collaborators_url":"https://api.github.com/repos/yola/yodeploy/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/yola/yodeploy/teams","hooks_url":"https://api.github.com/repos/yola/yodeploy/hooks","issue_events_url":"https://api.github.com/repos/yola/yodeploy/issues/events{/number}","events_url":"https://api.github.com/repos/yola/yodeploy/events","assignees_url":"https://api.github.com/repos/yola/yodeploy/assignees{/user}","branches_url":"https://api.github.com/repos/yola/yodeploy/branches{/branch}","tags_url":"https://api.github.com/repos/yola/yodeploy/tags","blobs_url":"https://api.github.com/repos/yola/yodeploy/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/yola/yodeploy/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/yola/yodeploy/git/refs{/sha}","trees_url":"https://api.github.com/repos/yola/yodeploy/git/trees{/sha}","statuses_url":"https://api.github.com/repos/yola/yodeploy/statuses/{sha}","languages_url":"https://api.github.com/repos/yola/yodeploy/languages","stargazers_url":"https://api.github.com/repos/yola/yodeploy/stargazers","contributors_url":"https://api.github.com/repos/yola/yodeploy/contributors","subscribers_url":"https://api.github.com/repos/yola/yodeploy/subscribers","subscription_url":"https://api.github.com/repos/yola/yodeploy/subscription","commits_url":"https://api.github.com/repos/yola/yodeploy/commits{/sha}","git_commits_url":"https://api.github.com/repos/yola/yodeploy/git/commits{/sha}","comments_url":"https://api.github.com/repos/yola/yodeploy/comments{/number}","issue_comment_url":"https://api.github.com/repos/yola/yodeploy/issues/comments{/number}","contents_url":"https://api.github.com/repos/yola/yodeploy/contents/{+path}","compare_url":"https://api.github.com/repos/yola/yodeploy/compare/{base}...{head}","merges_url":"https://api.github.com/repos/yola/yodeploy/merges","archive_url":"https://api.github.com/repos/yola/yodeploy/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/yola/yodeploy/downloads","issues_url":"https://api.github.com/repos/yola/yodeploy/issues{/number}","pulls_url":"https://api.github.com/repos/yola/yodeploy/pulls{/number}","milestones_url":"https://api.github.com/repos/yola/yodeploy/milestones{/number}","notifications_url":"https://api.github.com/repos/yola/yodeploy/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/yola/yodeploy/labels{/name}","releases_url":"https://api.github.com/repos/yola/yodeploy/releases{/id}","deployments_url":"https://api.github.com/repos/yola/yodeploy/deployments","created_at":"2012-06-27T14:06:15Z","updated_at":"2016-05-26T16:32:48Z","pushed_at":"2016-10-03T11:15:04Z","git_url":"git://github.com/yola/yodeploy.git","ssh_url":"git@github.com:yola/yodeploy.git","clone_url":"https://github.com/yola/yodeploy.git","svn_url":"https://github.com/yola/yodeploy","homepage":null,"size":533,"stargazers_count":2,"watchers_count":2,"language":"Python","has_issues":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"open_issues_count":17,"forks":0,"open_issues":17,"watchers":2,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/yola/yodeploy/pulls/177"},"html":{"href":"https://github.com/yola/yodeploy/pull/177"},"issue":{"href":"https://api.github.com/repos/yola/yodeploy/issues/177"},"comments":{"href":"https://api.github.com/repos/yola/yodeploy/issues/177/comments"},"review_comments":{"href":"https://api.github.com/repos/yola/yodeploy/pulls/177/comments"},"review_comment":{"href":"https://api.github.com/repos/yola/yodeploy/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/yola/yodeploy/pulls/177/commits"},"statuses":{"href":"https://api.github.com/repos/yola/yodeploy/statuses/dc3d4d6a1d61237dbef528b33319ff75683c02da"}}}}
{"actor":{"display_login":"jamesodo"}}