maintner: discard lock_reason in GitHub issue events

"lock_reason" is a field that was added to GitHub API v3 after maintner
was created. It's present in GitHub issue "locked" events and indicates
the reason the issue was locked, such as "off-topic", "too heated",
"resolved", etc.

Since maintner didn't have explicit handling for it, it got the default
treatment of being saved in OtherJSON just in case.

It's a non-goal for maintner to track all available data, only what is
deemed to be worth including. This field hasn't been used all this time
and seems fine to exclude until something changes.

Also fix a bug where OtherJSON wasn't even set in GitHubIssueEvent,
and improve various internal comments and log messages.

For golang/go#46180.

Change-Id: I9ebb3bf4183f78e413dfb032c94cb736a8b07e02
Reviewed-on: https://go-review.googlesource.com/c/build/+/320289
Trust: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Alexander Rakoczy <alex@golang.org>
This commit is contained in:
Dmitri Shuralyov 2021-05-14 17:19:23 -04:00
Родитель 0536f7398f
Коммит 39f255e9b9
2 изменённых файлов: 59 добавлений и 15 удалений

Просмотреть файл

@ -607,8 +607,8 @@ type GitHubIssueEvent struct {
Actor *GitHubUser
Label string // for type: "unlabeled", "labeled"
Assignee *GitHubUser // for type "assigned", "unassigned"
Assigner *GitHubUser // for type "assigned", "unassigned"
Assignee *GitHubUser // for type: "assigned", "unassigned"
Assigner *GitHubUser // for type: "assigned", "unassigned"
Milestone string // for type: "milestoned", "demilestoned"
From, To string // for type: "renamed"
CommitID, CommitURL string // for type: "closed", "referenced" ... ?
@ -703,6 +703,7 @@ func (r *GitHubRepo) newGithubEvent(p *maintpb.GithubIssueEvent) *GitHubIssueEve
// TODO: parse it and see if we've since learned how
// to deal with it?
log.Printf("Unknown JSON in log: %s", p.OtherJson)
e.OtherJSON = string(p.OtherJson)
}
if p.Label != nil {
e.Label = g.c.str(p.Label.Name)
@ -2130,8 +2131,8 @@ func (p *githubRepoPoller) syncEventsOnIssue(ctx context.Context, issueNum int32
return nil
}
// parseGithubEvents parses the JSON array of GitHub events in r. It
// does this the very manual way (using map[string]interface{})
// parseGithubEvents parses the JSON array of GitHub issue events in r.
// It does this the very manual way (using map[string]interface{})
// instead of using nice types because https://golang.org/issue/15314
// isn't implemented yet and also because even if it were implemented,
// this code still wants to preserve any unknown fields to store in
@ -2235,7 +2236,8 @@ func parseGithubEvents(r io.Reader) ([]*GitHubIssueEvent, error) {
e.TeamReviewer = t
}
}
delete(em, "node_id") // not sure what it is, but don't need to store it
delete(em, "node_id") // GitHub API v4 Global Node ID; don't store it.
delete(em, "lock_reason") // Not stored.
otherJSON, _ := json.Marshal(em)
e.OtherJSON = string(otherJSON)
@ -2243,7 +2245,7 @@ func parseGithubEvents(r io.Reader) ([]*GitHubIssueEvent, error) {
e.OtherJSON = ""
}
if e.OtherJSON != "" {
log.Printf("warning: storing unknown field(s) in GitHub event: %s", e.OtherJSON)
log.Printf("warning: storing unknown field(s) in GitHub issue event: %s", e.OtherJSON)
}
evts = append(evts, e)
}
@ -2336,7 +2338,7 @@ func (p *githubRepoPoller) syncReviewsOnPullRequest(ctx context.Context, issueNu
}
evts, err := parseGithubReviews(res.Body)
if err != nil {
return nil, nil, fmt.Errorf("%s: parse github pr review events: %v", u, err)
return nil, nil, fmt.Errorf("%s: parse github pr reviews: %v", u, err)
}
is := make([]interface{}, len(evts))
for i, v := range evts {
@ -2373,8 +2375,8 @@ func (p *githubRepoPoller) syncReviewsOnPullRequest(ctx context.Context, issueNu
return nil
}
// parseGithubReviews parses the JSON array of GitHub review events in r. It
// does this the very manual way (using map[string]interface{})
// parseGithubReviews parses the JSON array of GitHub reviews in r.
// It does this the very manual way (using map[string]interface{})
// instead of using nice types because https://golang.org/issue/15314
// isn't implemented yet and also because even if it were implemented,
// this code still wants to preserve any unknown fields to store in
@ -2437,7 +2439,7 @@ func parseGithubReviews(r io.Reader) ([]*GitHubReview, error) {
e.Created = e.Created.UTC()
}
delete(em, "node_id") // not sure what it is, but don't need to store it
delete(em, "node_id") // GitHub API v4 Global Node ID; don't store it.
delete(em, "html_url") // not needed.
delete(em, "pull_request_url") // not needed.
delete(em, "_links") // not needed. (duplicate data of above two nodes)
@ -2448,7 +2450,7 @@ func parseGithubReviews(r io.Reader) ([]*GitHubReview, error) {
e.OtherJSON = ""
}
if e.OtherJSON != "" {
log.Printf("warning: storing unknown field(s) in GitHub event: %s", e.OtherJSON)
log.Printf("warning: storing unknown field(s) in GitHub review: %s", e.OtherJSON)
}
evts = append(evts, e)
}

Просмотреть файл

@ -287,7 +287,8 @@ func TestParseGithubEvents(t *testing.T) {
"event": "locked",
"commit_id": null,
"commit_url": null,
"created_at": "2017-03-13T22:39:36Z"
"created_at": "2017-03-13T22:39:36Z",
"lock_reason": "off-topic"
}`,
e: &GitHubIssueEvent{
ID: 998144646,
@ -482,6 +483,47 @@ func TestParseGithubEvents(t *testing.T) {
RenameTo: "test-2 new name",
},
},
{
name: "Extra Unknown JSON",
j: `{
"id": 998144526,
"url": "https://api.github.com/repos/bradfitz/go-issue-mirror/issues/events/998144526",
"actor": {
"login": "bradfitz",
"id": 2621
},
"event": "labeled",
"commit_id": null,
"commit_url": null,
"created_at": "2017-03-13T22:39:28Z",
"label": {
"name": "enhancement",
"color": "84b6eb"
},
"random_new_field": "some new thing that GitHub API may add"
}
`,
e: &GitHubIssueEvent{
ID: 998144526,
Type: "labeled",
Created: t3339("2017-03-13T22:39:28Z"),
Actor: &GitHubUser{
ID: 2621,
Login: "bradfitz",
},
Label: "enhancement",
OtherJSON: `{"random_new_field":"some new thing that GitHub API may add"}`,
},
p: &maintpb.GithubIssueEvent{
Id: 998144526,
EventType: "labeled",
ActorId: 2621,
Created: p3339("2017-03-13T22:39:28Z"),
Label: &maintpb.GithubLabel{Name: "enhancement"},
OtherJson: []byte(`{"random_new_field":"some new thing that GitHub API may add"}`),
},
},
}
var eventTypes []string
@ -817,7 +859,7 @@ func TestParseGitHubReviews(t *testing.T) {
},
"submitted_at": "2018-03-22T00:26:48Z",
"commit_id" : "e4d70f7e8892f024e4ed3e8b99ee6c5a9f16e126",
"random_key": "some random value"
"random_new_field": "some new thing that GitHub API may add"
}`,
e: &GitHubReview{
ID: 123456,
@ -830,7 +872,7 @@ func TestParseGitHubReviews(t *testing.T) {
CommitID: "e4d70f7e8892f024e4ed3e8b99ee6c5a9f16e126",
ActorAssociation: "CONTRIBUTOR",
Created: t3339("2018-03-22T00:26:48Z"),
OtherJSON: `{"random_key":"some random value"}`,
OtherJSON: `{"random_new_field":"some new thing that GitHub API may add"}`,
},
p: &maintpb.GithubReview{
Id: 123456,
@ -840,7 +882,7 @@ func TestParseGitHubReviews(t *testing.T) {
CommitId: "e4d70f7e8892f024e4ed3e8b99ee6c5a9f16e126",
ActorAssociation: "CONTRIBUTOR",
Created: p3339("2018-03-22T00:26:48Z"),
OtherJson: []byte(`{"random_key":"some random value"}`),
OtherJson: []byte(`{"random_new_field":"some new thing that GitHub API may add"}`),
},
},
}