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

Update GHEvent.java #1233

Merged
merged 2 commits into from
Sep 24, 2021
Merged

Update GHEvent.java #1233

merged 2 commits into from
Sep 24, 2021

Conversation

philippebailer
Copy link
Contributor

@philippebailer philippebailer commented Sep 21, 2021

Description

Seems we are rehashing this issue: https://issues.jenkins.io/browse/JENKINS-63348

Which was fixed by @bitwiseman with this #911

I'm similarly adding Pull request review thread and Merge queue entry events to avoid breaking auth as unlike the previous ticket unsubscribing from Merge queue entry does not seem to work. These events seem to be new since I have found no documentation on them at webhooks-and-events

We just finished setting up the github app yesterday to have it break a few hrs later with the merge event error so I assume some change regarding that event was released at that time.

Idk if its possible to get this change merged and pushed through to a new version of github-api-plugin but our jenkins integration setup is currently blocked by this. Please let me know if there is anything I can do to help move this along.

Thanks you!

Before submitting a PR:

We love getting PRs, but we hate asking people for the same basic changes every time.

  • Push your changes to a branch other than main. Create your PR from that branch.
  • Add JavaDocs and other comments
  • Write tests that run and pass in CI. See CONTRIBUTING.md for details on how to capture snapshot data.
  • Run mvn -D enable-ci clean install site locally. If this command doesn't succeed, your change will not pass CI.

When creating a PR:

  • Fill in the "Description" above.
  • Enable "Allow edits from maintainers".

Closes #1234

seems we are rehashing this issue: https://issues.jenkins.io/browse/JENKINS-63348

adding `Pull request review thread` and `Merge queue entry` events to avoid breaking auth
@philippebailer
Copy link
Contributor Author

For some additional data the errors I've experienced are:

com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type `org.kohsuke.github.GHEvent` from String "pull_request_review_thread": not one of the values accepted for Enum class: [GOLLUM, ALL, INTEGRATION_INSTALLATION_REPOSITORIES, META, DOWNLOAD, FORK_APPLY, CHECK_RUN, MARKETPLACE_PURCHASE, PROJECT_CARD, PUSH, PAGE_BUILD, TEAM_ADD, REPOSITORY_VULNERABILITY_ALERT, PACKAGE, DELETE, DEPLOYMENT_STATUS, WATCH, FOLLOW, ORG_BLOCK, ORGANIZATION, COMMIT_COMMENT, ISSUE_COMMENT, REPOSITORY_IMPORT, GITHUB_APP_AUTHORIZATION, CODE_SCANNING_ALERT, CHECK_SUITE, FORK, GIST, CONTENT_REFERENCE, REPOSITORY, INSTALLATION_REPOSITORIES, REPOSITORY_DISPATCH, MILESTONE, STAR, LABEL, MEMBERSHIP, SECURITY_ADVISORY, PROJECT_COLUMN, TEAM, PULL_REQUEST_REVIEW, RELEASE, PUBLIC, WORKFLOW_RUN, STATUS, PULL_REQUEST, PROJECT, WORKFLOW_DISPATCH, CREATE, MEMBER, REGISTRY_PACKAGE, INSTALLATION, DEPLOYMENT, PULL_REQUEST_REVIEW_COMMENT, DEPLOY_KEY, PING, ISSUES]
 at [Source: (String)"{"id":....

and

com.fasterxml.jackson.databind.exc.InvalidFormatException: Cannot deserialize value of type `org.kohsuke.github.GHEvent` from String "merge_queue_entry": not one of the values accepted for Enum class: [GOLLUM, ALL, INTEGRATION_INSTALLATION_REPOSITORIES, META, DOWNLOAD, FORK_APPLY, CHECK_RUN, MARKETPLACE_PURCHASE, PROJECT_CARD, PUSH, PAGE_BUILD, TEAM_ADD, REPOSITORY_VULNERABILITY_ALERT, PACKAGE, DELETE, DEPLOYMENT_STATUS, WATCH, FOLLOW, ORG_BLOCK, ORGANIZATION, COMMIT_COMMENT, ISSUE_COMMENT, REPOSITORY_IMPORT, GITHUB_APP_AUTHORIZATION, CODE_SCANNING_ALERT, CHECK_SUITE, FORK, GIST, CONTENT_REFERENCE, REPOSITORY, INSTALLATION_REPOSITORIES, REPOSITORY_DISPATCH, MILESTONE, STAR, LABEL, MEMBERSHIP, SECURITY_ADVISORY, PROJECT_COLUMN, TEAM, PULL_REQUEST_REVIEW, RELEASE, PUBLIC, WORKFLOW_RUN, STATUS, PULL_REQUEST, PROJECT, WORKFLOW_DISPATCH, CREATE, MEMBER, REGISTRY_PACKAGE, INSTALLATION, DEPLOYMENT, PULL_REQUEST_REVIEW_COMMENT, DEPLOY_KEY, PING, ISSUES]
 at [Source: (String)"[{"id":......

I can reproduce the merge_queue_entry error regardless of what events are selected, even no event subscriptions will generate the above error and block the jenkins plugin from continuing

@philippebailer
Copy link
Contributor Author

It would also be great to just support unknown values as shown here

Mapper change at GitHubClient

MAPPER.configure(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE, true);

Enum change at GHEvent

@JsonEnumDefaultValue 
UNKNOWN

If the above seems fine I can open another PR with those changes to see if any tests run into issues.

I don't want to make this change in this PR since I really just want to get my integration working and the above solution seems like the most straight forward option.

@bitwiseman
Copy link
Member

It would also be great to just support unknown values as shown here

Mapper change at GitHubClient

MAPPER.configure(DeserializationFeature.READ_UNKNOWN_ENUM_VALUES_USING_DEFAULT_VALUE, true);

Enum change at GHEvent

@JsonEnumDefaultValue 
UNKNOWN

If the above seems fine I can open another PR with those changes to see if any tests run into issues.

I don't want to make this change in this PR since I really just want to get my integration working and the above solution seems like the most straight forward option.

@philippebailer
We chose to go a different direction. Instead of serializing directly enum, we've moved to serializing to string and the converting to enum on read. This perserves the returned value for analysis if needed.

Updated expected count for event enum.
@codecov
Copy link

codecov bot commented Sep 24, 2021

Codecov Report

Merging #1233 (656f5de) into main (bcb71a3) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #1233   +/-   ##
=========================================
  Coverage     76.90%   76.91%           
  Complexity     1885     1885           
=========================================
  Files           188      188           
  Lines          5958     5960    +2     
  Branches        328      328           
=========================================
+ Hits           4582     4584    +2     
  Misses         1176     1176           
  Partials        200      200           
Impacted Files Coverage Δ
src/main/java/org/kohsuke/github/GHEvent.java 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bcb71a3...656f5de. Read the comment docs.

@bitwiseman bitwiseman merged commit 1b926cc into hub4j:main Sep 24, 2021
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.

GHEvent is missing new events that is breaking Jenkins plugin usage
3 participants