Skip to content
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

baldrick pulls config from fork's master branch #111

Open
pllim opened this issue Apr 5, 2021 · 7 comments
Open

baldrick pulls config from fork's master branch #111

pllim opened this issue Apr 5, 2021 · 7 comments
Labels
bug Something isn't working

Comments

@pllim
Copy link
Contributor

pllim commented Apr 5, 2021

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.

@pllim pllim added the bug Something isn't working label Apr 5, 2021
@Cadair
Copy link
Member

Cadair commented Apr 5, 2021

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
Copy link
Contributor Author

pllim commented Apr 5, 2021

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.

@Cadair
Copy link
Member

Cadair commented Apr 6, 2021

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

@Cadair
Copy link
Member

Cadair commented Apr 6, 2021

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
Copy link
Contributor Author

pllim commented Apr 6, 2021

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
Copy link
Contributor Author

pllim commented Apr 6, 2021

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
Copy link
Contributor Author

pllim commented Apr 6, 2021

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': ...}}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants