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

In sync tool, fix "existing PR" query: consider repository name #127

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

dagood
Copy link
Member

@dagood dagood commented Jun 19, 2024

Fixes an error in the sync pipeline that occurred because two active PRs into branches named microsoft/main were happening at the same time, microsoft/go#1251 and microsoft/go-images#316:

https://dev.azure.com/dnceng/internal/_build/results?buildId=2477557&view=logs&j=881b2ec9-fc45-57ee-6456-bdd38ea7f007&t=a85e175a-b476-51c2-71e7-c3450accbe07&l=201

The GraphQL API doesn't have a filter for repository name, and the job got this data:

{
  "data": {
    "user": {
      "pullRequests": {
        "nodes": [
          {
            "title": "Update submodule to latest `master` in `microsoft/main`",
            "id": "PR_kwDOFG13585yf6nr",
            "number": 1251,
            "headRepositoryOwner": {
              "login": "microsoft"
            },
            "baseRepository": {
              "owner": {
                "login": "microsoft"
              }
            }
          },
          {
            "title": "Update submodule to latest `master` in `microsoft/main`",
            "id": "PR_kwDOF7mTR85yf6pG",
            "number": 316,
            "headRepositoryOwner": {
              "login": "microsoft"
            },
            "baseRepository": {
              "owner": {
                "login": "microsoft"
              }
            }
          }
        ]
      }
    }
  }
}

The sync code has a safeguard to make sure only 1 or 0 results were returned to make sure that it doesn't get mixed up. That was triggered and the sync pipeline failed safe:

expected 0/1 PR search result, found 2
failed to submit one or more PRs
=== Failed sync 2/2

#124 uncovered this issue because, for the first time, the synced branch names used in multiple repos overlapped.

This PR fixes it by adding an extra field that GraphQL should return from the query, nameWithOwner. (microsoft/go, microsoft/go-images.) It's used to filter the results down. I used https://docs.github.com/en/graphql/overview/explorer to find and try out this new field.

@dagood dagood requested a review from a team as a code owner June 19, 2024 20:13
@dagood
Copy link
Member Author

dagood commented Jun 19, 2024

@dagood dagood merged commit 9c02a28 into main Jun 19, 2024
10 checks passed
@dagood dagood deleted the dev/dagood/fix-existing-pr-query branch June 19, 2024 20:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants