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

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

Merged
merged 1 commit into from
Sep 17, 2019

Conversation

McFoggy
Copy link
Contributor

@McFoggy McFoggy commented Apr 1, 2019

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

Copy link
Member

@KostyaSha KostyaSha left a comment

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

McFoggy commented Sep 6, 2019

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

Copy link
Member

@KostyaSha KostyaSha left a comment

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

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
3 participants