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

GHPullRequest.getLabels should not go to the GHIssue API endpoint #964

Merged
merged 1 commit into from
Oct 22, 2020

Conversation

jglick
Copy link
Contributor

@jglick jglick commented Oct 15, 2020

@jvz found that the Jenkins label filter plugin does not work with App authentication. After jenkinsci/github-branch-source-plugin#351 + jenkinsci/github-label-filter-plugin#4 I think I figured out the problem: the lookup of PR labels—and only this method—uses https://developer.github.com/v3/issues/#get-an-issue rather than https://developer.github.com/v3/pulls/#get-a-pull-request as you might expect. Presumably the fact that the App is configured to permit https://developer.github.com/v3/apps/permissions/#permission-on-pull-requests but not https://developer.github.com/v3/apps/permissions/#permission-on-issues makes this not work. I tracked this weird logic down to 372d5ff from 2014 which has little explanation and may have been a workaround for some transient bug in GH long since fixed; I get the expected metadata today with either endpoint (using a PAT):

$ curl -u $AUTH -s https://api.github.com/repos/jenkinsci/mercurial-plugin/issues/150 | jq .labels
[
  {
    "id": 1447091401,
    "node_id": "MDU6TGFiZWwxNDQ3MDkxNDAx",
    "url": "https://api.github.com/repos/jenkinsci/mercurial-plugin/labels/test",
    "name": "test",
    "color": "1c6c87",
    "default": false,
    "description": ""
  }
]
$ curl -u $AUTH -s https://api.github.com/repos/jenkinsci/mercurial-plugin/pulls/150 | jq .labels
[
  {
    "id": 1447091401,
    "node_id": "MDU6TGFiZWwxNDQ3MDkxNDAx",
    "url": "https://api.github.com/repos/jenkinsci/mercurial-plugin/labels/test",
    "name": "test",
    "color": "1c6c87",
    "default": false,
    "description": ""
  }
]

Possibly fixes jenkinsci/github-label-filter-plugin#3 (not yet verified).

@jvz
Copy link

jvz commented Oct 15, 2020

Tested, works for me and fixes the problem.

@francais01
Copy link

FYI: I wanted to give this a test, but I ran into an error. I cloned your repo, installed maven (I'm not a Java dev) and applied this patch to pom.xml:

diff --git a/pom.xml b/pom.xml
index 440de340..74e62c96 100644
--- a/pom.xml
+++ b/pom.xml
@@ -32,6 +32,15 @@
 
   <build>
     <plugins>
+      <plugin>
+        <groupId>org.apache.maven.plugins</groupId>
+        <artifactId>maven-compiler-plugin</artifactId>
+        <version>3.6.0</version>
+        <configuration>
+          <source>1.6</source>
+          <target>1.6</target>
+        </configuration>
+      </plugin>
       <plugin>
         <groupId>com.infradna.tool</groupId>
         <artifactId>bridge-method-injector</artifactId>

However, the build still failed with this error:

[ERROR] java.nio.file.NoSuchFileException: /home/olivier/github-api/target/classes/META-INF/annotations/com.infradna.tool.bridge_method_injector.WithBridgeMethods

I'm on openSUSE Leap 15.2 with this java -version:

openjdk version "11.0.8" 2020-07-14
OpenJDK Runtime Environment (build 11.0.8+10-suse-lp152.2.3.1-x8664)
OpenJDK 64-Bit Server VM (build 11.0.8+10-suse-lp152.2.3.1-x8664, mixed mode)

@jglick
Copy link
Contributor Author

jglick commented Oct 19, 2020

@francais01 well…do not apply that patch. Not sure why you thought you should.

Let me see if I can get a draft PR on the wrapper plugin to deploy a build suitable for testing.

@jglick
Copy link
Contributor Author

jglick commented Oct 19, 2020

@jglick
Copy link
Contributor Author

jglick commented Oct 19, 2020

@timja timja added the bug label Oct 22, 2020
@timja timja requested a review from bitwiseman October 22, 2020 20:02
@jglick
Copy link
Contributor Author

jglick commented Oct 22, 2020

Unfortunately @bitwiseman is not available, and it is not clear there are any other maintainers to step in during his absence.

@timja
Copy link
Collaborator

timja commented Oct 22, 2020

Unfortunately @bitwiseman is not available, and it is not clear there are any other maintainers to step in during his absence.

I can merge here but I don't have release permissions unfortunately.
How long is he away for? or unknown?

@timja timja merged commit 5377d0d into hub4j:master Oct 22, 2020
@jglick jglick deleted the GHPullRequest.getLabels branch October 22, 2020 21:43
@bitwiseman
Copy link
Member

I'm back. I can cut a release this week.

*/
public Collection<GHLabel> getLabels() throws IOException {
public Collection<GHLabel> getLabels() {
Copy link
Member

Choose a reason for hiding this comment

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

@jglick @timja
This is an API change, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes; binary compatible, potentially source-incompatible.

@jglick
Copy link
Contributor Author

jglick commented Nov 24, 2020

I can cut a release this week.

@bitwiseman ping?

BTW for some reason https://github.com/hub4j/github-api/releases does not list this PR in the draft section.

@bitwiseman
Copy link
Member

@jglick
Done. It will take a little while for it to propagate

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Does not find any labels
5 participants