OpenAstronomy/baldrick

baldrick pulls config from fork's master branch

pllim opened this issue · 7 comments

pllim commented
File ".../flask/_compat.py", line 39, in reraise
  raise value
        └ AssertionError(b'{"message":"No commit found for the ref master","documentation_url":...
...
File ".../baldrick/github/github_api.py", line 282, in get_file_contents
  return super().get_file_contents(path_to_file, branch=branch)
                                    │                    └ 'master'
                                    └ 'pyproject.toml'
File ".../baldrick/github/github_api.py", line 89, in get_file_contents
  assert response.ok, response.content
         │        │   │        └ <property object at 0x7f51b612ce50>
         │        │   └ <Response [404]>
         │        └ <property object at 0x7f51b612cc70>
         └ <Response [404]>

The above log, combined with what I saw in astropy/astropy#11481

baldrick.github.github_api:get_repo_config:144 - Got this combined config from
nstarman/astropy@master:

suggests that baldrick is attempting to read the config from a fork's master, not upstream.

This would explain why it stopped running for my PRs because I deleted my fork's master branch a long time ago, as suggested in https://docs.astropy.org/en/latest/development/workflow/development_workflow.html#deleting-your-master-branch .

And I clearly remember that the bot used to work despite my fork not having a master branch before but I don't remember when it stopped working. I just know it's been that way (broken) for quite some time; just didn't know why until now.

Huh, it should be pulling from whatever GH gives us as the base branch: https://github.com/OpenAstronomy/baldrick/blob/master/baldrick/github/github_api.py#L781

pllim commented

Apparently base branch of the PR is the default branch of upstream of the fork but it then tries to pull the file from that branch but from the fork. I attempted a fix at #112 but can use some help in getting the tests to pass.

I had a look at your PR, and I think it's fixing a symptom not the actual bug. According to everything I can find online, a Pull Request event is triggered from the upstream repo not the fork from which it originates. So all the GH API classes which get created in the event handling code should be pointing at upstream not the fork, and we should be using the base branch of the upstream as the place to pull the config.

I think what we need to work out is why we are not alway getting the upstream when we create the repo object here: https://github.com/OpenAstronomy/baldrick/blob/master/baldrick/blueprints/github.py#L43

I also can't reproduce this bug on sunpy/Giles. I am wondering if you are seeing this on all astropy PRs or if something is wonky with just that one / user or something.

pllim commented

Why would it have exception rules for a few users? Maybe we can do a print on Line 43 for debugging. The problem with Heroku log is it is too opaque.

pllim commented

So, this is the log after merging astropy/astropy#11483 . I still don't understand it. Seems to grab config twice, once from fork and once from upstream. And whatever happened didn't kick off the towncrier check. I think this event was tied to astropy/astropy#11456

2021-04-06T14:15:25.792802+00:00 app[web.1]: 2021-04-06 14:15:25.792 | DEBUG    |
baldrick.github.github_api:get_repo_config:144 - Got this combined config from WilliamJamieson/astropy@master:
{'autolabel': {'enabled': False},
 'changelog_checker': {'enabled': False},
 'milestones': {'enabled': True},
 'pull_requests': {'enabled': True,
                   'post_pr_comment': False,
                   'skip_labels': ['Experimental', 'Work in progress'],
                   'skip_fails': True,
                   'pull_request_substring': 'issues related to the changelog'}}
2021-04-06T14:15:25.792818+00:00 app[web.1]: Skipping PR changelog check, disabled in config.
...
2021-04-06T14:15:26.177164+00:00 app[web.1]: 2021-04-06 14:15:26.176 | DEBUG    |
baldrick.github.github_api:get_repo_config:144 - Got this combined config from astropy/astropy@master:
{'autolabel': {'enabled': False},
 'changelog_checker': {'enabled': False},
 'milestones': {'enabled': True},
 'pull_requests': {'enabled': True,
                   'post_pr_comment': False,
                   'skip_labels': ['Experimental', 'Work in progress'],
                   'skip_fails': True,
                   'pull_request_substring': 'issues related to the changelog'},
'towncrier_changelog': {'enabled': True,
                        'verify_pr_number': True,
                        'changelog_skip_label': 'no-changelog-entry-needed',
                        'help_url': 'https://github.com/astropy/astropy/blob/master/docs/changes/README.rst',
                        'changelog_missing_long': "There isn't a changelog file in this pull request.
Please add a changelog file to the `changelog/` directory following the instructions
in the changelog [README](https://github.com/astropy/astropy/blob/master/docs/changes/README.rst).",
                        'type_incorrect_long': 'The changelog file you added is not one of the allowed types.
Please use one of the types described in the changelog
[README](https://github.com/astropy/astropy/blob/master/docs/changes/README.rst)',
                        'number_incorrect_long': 'The number in the changelog file you added
does not match the number of this pull request. Please rename the file.'}}
pllim commented

And this is the log after I asked PR author to rebase and push, and still nothing on the PR check statuses (statii?).

2021-04-06T14:46:03.950169+00:00 app[web.1]: 2021-04-06 14:46:03.949 | DEBUG    |
baldrick.github.github_api:get_repo_config:144 - Got this combined config from WilliamJamieson/astropy@master:
{'autolabel': {'enabled': False},
 'changelog_checker': {'enabled': False},
 'milestones': {'enabled': True},
 'pull_requests': {'enabled': True,
                   'post_pr_comment': False,
                   'skip_labels': ['Experimental', 'Work in progress'],
                   'skip_fails': True,
                   'pull_request_substring': 'issues related to the changelog'},
 'towncrier_changelog': {'enabled': True,
                         'verify_pr_number': True,
                         'changelog_skip_label': 'no-changelog-entry-needed',
                         'help_url': 'https://github.com/astropy/astropy/blob/master/docs/changes/README.rst',
                         'changelog_missing_long': ...,
                         'type_incorrect_long': ...,
                         'number_incorrect_long': ...}}
2021-04-06T14:46:03.950270+00:00 app[web.1]: Skipping PR changelog check, disabled in config.
...
2021-04-06T14:46:04.350450+00:00 app[web.1]: 2021-04-06 14:46:04.350 | DEBUG    |
baldrick.github.github_api:get_repo_config:144 - Got this combined config from astropy/astropy@master:
{'autolabel': {'enabled': False},
 'changelog_checker': {'enabled': False},
 'milestones': {'enabled': True},
 'pull_requests': {'enabled': True,
                   'post_pr_comment': False,
                   'skip_labels': ['Experimental', 'Work in progress'],
                   'skip_fails': True,
                   'pull_request_substring': 'issues related to the changelog'},
'towncrier_changelog': {'enabled': True,
                        'verify_pr_number': True,
                        'changelog_skip_label': 'no-changelog-entry-needed',
                        'help_url': 'https://github.com/astropy/astropy/blob/master/docs/changes/README.rst',
                        'changelog_missing_long': ...,
                        'type_incorrect_long': ...,
                        'number_incorrect_long': ...}}