Revert "don't commit on behalf of user, commit with the bot"

This reverts commit deee6b6.

pyup/bot.py

@@ -114,10 +114,6 @@ def can_pull(self, initial, scheduled):
 
     def apply_updates(self, initial, scheduled):
 
-        # make sure that the bot is a collaborator (if run by a bot)
-        if self.bot_token:
-            self.provider.get_pull_request_permissions(user=self.bot, repo=self.user_repo)
-
         InitialUpdateClass = self.req_bundle.get_initial_update_class()
 
         if initial:
@@ -293,7 +289,7 @@ def create_branch(self, new_branch, delete_empty=False):
             self.provider.create_branch(
                 base_branch=self.config.branch,
                 new_branch=new_branch,
-                repo=self.bot_repo if self.bot_token else self.user_repo
+                repo=self.user_repo
             )
             logger.info("Created branch {} from {}".format(new_branch, self.config.branch))
             return True
@@ -327,21 +323,23 @@ def pull_config(self, new_config):  # pragma: no cover
                 logger.info(
                     "Config file exists, updating config for sha {}".format(content_file.sha))
                 commit = self.provider.create_commit(
-                    repo=self.bot_repo if self.bot_token else self.user_repo,
+                    repo=self.user_repo,
                     path="/.pyup.yml",
                     branch=branch,
                     content=content,
                     commit_message="update pyup.io config file",
+                    committer=self.bot if self.bot_token else self.user,
                     sha=content_file.sha
                 )
             logger.info("No config file found, writing new config file")
             # there's no config file present, write a new config file and commit it
             commit = self.provider.create_and_commit_file(
-                repo=self.bot_repo if self.bot_token else self.user_repo,
+                repo=self.user_repo,
                 path="/.pyup.yml",
                 branch=branch,
                 content=content,
                 commit_message="create pyup.io config file",
+                committer=self.bot if self.bot_token else self.user,
             )
 
             title = 'Config file for pyup.io'
@@ -376,12 +374,13 @@ def commit_and_pull(self, initial, new_branch, title, body, updates):
                 content = update.requirement.update_content(content, self.config.update_hashes)
                 if content != old_content:
                     new_sha = self.provider.create_commit(
-                        repo=self.bot_repo if self.bot_token else self.user_repo,
+                        repo=self.user_repo,
                         path=update.requirement_file.path,
                         branch=new_branch,
                         content=content,
                         commit_message=update.commit_message,
                         sha=sha,
+                        committer=self.bot if self.bot_token else self.user,
                     )
                     updated_files[update.requirement_file.path] = {"sha": new_sha,
                                                                    "content": content}
@@ -409,6 +408,23 @@ def create_issue(self, title, body):
 
     def create_pull_request(self, title, body, new_branch):
 
+        # if we have a bot user that creates the PR, we might run into problems on private
+        # repos because the bot has to be a collaborator. We try to submit the PR before checking
+        # the permissions because that saves us API calls in most cases
+        if self.bot_token:
+            try:
+                return self.provider.create_pull_request(
+                    repo=self.bot_repo,
+                    title=title,
+                    body=body,
+                    base_branch=self.config.branch,
+                    new_branch=new_branch,
+                    pr_label=self.config.label_prs,
+                    assignees=self.config.assignees
+                )
+            except NoPermissionError:
+                self.provider.get_pull_request_permissions(self.bot, self.user_repo)
+
         return self.provider.create_pull_request(
             repo=self.bot_repo if self.bot_token else self.user_repo,
             title=title,

pyup/providers/github.py

@@ -72,19 +72,14 @@ def get_file(self, repo, path, branch):
             ))
             return None, None
 
-    def create_and_commit_file(self, repo, path, branch, content, commit_message):
-        try:
-            return repo.create_file(
-                path=path,
-                message=commit_message,
-                content=content,
-                branch=branch,
-            )
-        except GithubException as e:  # pragma: no cover
-            # if the error status is 404 we might need to add the bot as collaborator
-            if e.status == 404:
-                raise NoPermissionError
-            raise e
+    def create_and_commit_file(self, repo, path, branch, content, commit_message, committer):
+        return repo.create_file(
+            path=path,
+            message=commit_message,
+            content=content,
+            branch=branch,
+            committer=self.get_committer_data(committer),
+        )
 
     def get_requirement_file(self, repo, path, branch):
         content, file_obj = self.get_file(repo, path, branch)
@@ -134,7 +129,7 @@ def delete_branch(self, repo, branch, prefix):
         ref = repo.get_git_ref("/".join(["heads", branch]))
         ref.delete()
 
-    def create_commit(self, path, branch, commit_message, content, sha, repo):
+    def create_commit(self, path, branch, commit_message, content, sha, repo, committer):
         # there's a rare bug in the github API when committing too fast on really beefy
         # hardware with Gigabit NICs (probably because they do some async stuff).
         # If we encounter an error, the loop waits for 1/2/3 seconds before trying again.
@@ -150,20 +145,15 @@ def create_commit(self, path, branch, commit_message, content, sha, repo):
                     content=content,
                     branch=branch,
                     sha=sha,
+                    committer=self.get_committer_data(committer),
                 )
                 return data["content"].sha
-            except GithubException as e:  # pragma: no cover
-                # if the error status is 404 we might need to add the bot as collaborator
-                if e.status == 404:
-                    raise NoPermissionError
-                elif e.status == 409:
-                    if i == 6:
-                        logger.error("Unable to create commit on {repo} for path {path}".format(
-                            repo=repo,
-                            path=path
-                        ), exc_info=True)
-                        raise e
-                else:
+            except GithubException as e:
+                if i == 6:
+                    logger.error("Unable to create commit on {repo} for path {path}".format(
+                        repo=repo,
+                        path=path
+                    ), exc_info=True)
                     raise e
                 time.sleep(i)
 

tests/test_bot.py

@@ -599,6 +599,61 @@ def test_bot_no_errors(self):
         })
         self.assertEqual(bot.provider.get_pull_request_permissions.called, False)
 
+    def test_bot_permission_error_resolved(self):
+        bot = bot_factory(bot_token="foo")
+        bot.provider.create_pull_request.side_effect = [NoPermissionError, "the foo"]
+        bot._bot_repo = "BOT REPO"
+        bot._user_repo = "USER REPO"
+        bot.create_pull_request("title", "body", "new_branch")
+        self.assertEqual(bot.provider.create_pull_request.called, True)
+        self.assertEqual(bot.provider.create_pull_request.call_args_list[0][1], {
+            "base_branch": "base_branch",
+            "new_branch": "new_branch",
+            "repo": "BOT REPO",
+            "body": "body",
+            "title": "title",
+            "pr_label": False,
+            "assignees": []
+        })
+        self.assertEqual(bot.provider.create_pull_request.call_args_list[1][1], {
+            "base_branch": "base_branch",
+            "new_branch": "new_branch",
+            "repo": "BOT REPO",
+            "body": "body",
+            "title": "title",
+            "pr_label": False,
+            "assignees": []
+
+        })
+
+    def test_bot_permission_error_not_resolved(self):
+        bot = bot_factory(bot_token="foo")
+        bot.provider.create_pull_request.side_effect = [NoPermissionError, NoPermissionError]
+        bot._bot_repo = "BOT REPO"
+        bot._user_repo = "USER REPO"
+        with self.assertRaises(NoPermissionError):
+            bot.create_pull_request("title", "body", "new_branch")
+        self.assertEqual(bot.provider.create_pull_request.called, True)
+        self.assertEqual(bot.provider.create_pull_request.call_args_list[0][1], {
+            "base_branch": "base_branch",
+            "new_branch": "new_branch",
+            "repo": "BOT REPO",
+            "body": "body",
+            "title": "title",
+            "pr_label": False,
+            "assignees": []
+        })
+        self.assertEqual(bot.provider.create_pull_request.call_args_list[1][1], {
+            "base_branch": "base_branch",
+            "new_branch": "new_branch",
+            "repo": "BOT REPO",
+            "body": "body",
+            "title": "title",
+            "pr_label": False,
+            "assignees": []
+        })
+
+
 class CloseStalePRsTestCase(TestCase):
 
     def setUp(self):

tests/test_github.py

@@ -139,12 +139,13 @@ def test_delete_branch(self):
     def test_create_commit(self, time):
         self.repo.update_file.return_value = {"commit": Mock(), "content": Mock()}
         self.provider.get_committer_data = Mock(return_value = "foo@bar.com")
-        self.provider.create_commit("path", "branch", "commit", "content", "sha", self.repo)
+        self.provider.create_commit("path", "branch", "commit", "content", "sha", self.repo, "com")
         self.assertEquals(self.repo.update_file.call_count, 1)
 
         self.repo.update_file.side_effect = GithubException(data="", status=1)
         with self.assertRaises(GithubException):
-            self.provider.create_commit("path", "branch", "commit", "content", "sha", self.repo)
+            self.provider.create_commit("path", "branch", "commit", "content", "sha", self.repo,
+                                        "com")
 
     def test_create_and_commit_file(self):
         repo = Mock()
@@ -164,12 +165,14 @@ def test_create_and_commit_file(self):
             commit_message=commit_message,
             branch=branch,
             content=content,
+            committer=committer
         )
         repo.create_file.assert_called_once_with(
             path=path,
             message=commit_message,
             content=content,
             branch=branch,
+            committer='committer-data'
         )
 
     def test_get_committer_data(self):