Skip to content

[JENKINS-56830] alternative use of 'html_url' in payload of event - #212

Merged
KostyaSha merged 1 commit into
jenkinsci:masterfrom
McFoggy:htmlUrl
Sep 17, 2019
Merged

[JENKINS-56830] alternative use of 'html_url' in payload of event#212
KostyaSha merged 1 commit into
jenkinsci:masterfrom
McFoggy:htmlUrl

Conversation

@McFoggy

@McFoggy McFoggy commented Apr 1, 2019

Copy link
Copy Markdown
Contributor

As documented in github API docs (https://developer.github.com/v3/repos/#response) the url field of the event should/could point to the API endpoint.
In the github-plugin it is expected to work on the public html url which is handled by the 'html_url' field of the event.
This commit thus try as before to build a GitHubRepositoryName from the 'url' field ; if that fails, as a fallback it also tries the 'html_url' field.
See also https://github.community/t5/GitHub-API-Development-and/consistency-of-repository-url-between-event-types/td-p/21209 for some explanations.


This change is Reviewable

@KostyaSha KostyaSha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logger

As documented in github API docs (https://developer.github.com/v3/repos/#response) the url field of the event should/could point to the API endpoint.
In the github-plugin it is expected to work on the public html url which is handled by the 'html_url' field of the event.
This commit thus try as before to build a GitHubRepositoryName from the 'url' field ; if that fails, as a fallback it also tries the 'html_url' field.
See also https://github.community/t5/GitHub-API-Development-and/consistency-of-repository-url-between-event-types/td-p/21209 for some explanations.
@McFoggy

McFoggy commented Sep 6, 2019

Copy link
Copy Markdown
Contributor Author

@KostyaSha modification done as as requested ; please look at latest push. Thx

@McFoggy
McFoggy requested a review from KostyaSha September 16, 2019 12:47

@KostyaSha KostyaSha left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@KostyaSha
KostyaSha merged commit 73e2513 into jenkinsci:master Sep 17, 2019
@McFoggy
McFoggy deleted the htmlUrl branch September 18, 2019 06:19
@solvingj

Copy link
Copy Markdown

This should have printed it's message with Logger.warning() because currently, if this alternate path is taken, the last message in the default system log is:

WARNING c.c.jenkins.GitHubRepositoryName#create: Could not match URL

Which would seem like it's the end of the road for this PUSH hook being processed. But, adding a custom logger and turning up the debug level, we can see this happening after:

PushEvent handling: 'html_url' field has been used to retrieve project information (instead of default 'url' field)

Which indicates that in-fact, we did in-fact successfully create a GitHubRepositoryName and assign it to fromEventRepository.

Unfortunately, builds still do not trigger on this path. But, this comment is only to suggest changing the log level here.

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.

3 participants