vsoch / pull-request-action

open a pull request when a branch is pushed or updated
https://github.com/marketplace/actions/pull-request-action
MIT License
174 stars 62 forks source link

Fix Auth: Change token to Bearer #17

Closed JasCodes closed 4 years ago

vsoch commented 4 years ago

Could you please explain this change? The documentation explicitly says to use token:

https://developer.github.com/v3/#authentication

JasCodes commented 4 years ago

https://help.github.com/en/actions/automating-your-workflow-with-github-actions/authenticating-with-the-github_token Is master working in your actions? Because it wasn't for me. Based on above link, it got fixed for me.

Thx

vsoch commented 4 years ago

Could you please link me to the failing action? I did try this recently and it worked ok. I’ll need to contact GitHub support to ask about the discrepancy.

vsoch commented 4 years ago

See here, this was run recently when the action was updated for the marketplace https://github.com/vsoch/pull-request-action-example/pull/1

vsoch commented 4 years ago

hey @jascodes I just submit a GitHub support request - hopefully this will shed some light, and I'd be happy to update the header string once they confirm which documentation is correct (and when the change happened to render the previously working header no longer valid). Thanks for taking the initiative to fix this up, I'll ping again when I hear back.

JasCodes commented 4 years ago

https://github.com/jascodes/ga/runs/435634825?check_suite_focus=true

8th command name Generate pull request

Then I forked your action and used patched version, https://github.com/jascodes/ga/runs/435687846?check_suite_focus=true

vsoch commented 4 years ago

Sorry - did you reverse those? The top one (the current?) seems okay (and what I've seen) the second "patched version" (what is here?) has the error.

vsoch commented 4 years ago

Oh I see, the branches referenced are at the top. That definitely confirms there was a failure - I still want to hear from GitHub about why/when the change happened, but we will get this merged soon.

vsoch commented 4 years ago

Could you please show that vsoch/pull-request-action@master also fails? The master is from your branch.

JasCodes commented 4 years ago

The bellow commit uses vsoch/pull-request-action@master https://github.com/jascodes/ga/runs/435634825?check_suite_focus=true

and when you check the log on command Enter host password for user 'jascodes':

This meant that the auth failed

vsoch commented 4 years ago

I'm confused though - the outcome that you are showing in the second link shows that the pull request request was successful. This is different than what you showed me on your master earlier where it was a clear failure. Can you please explain what is broken about the current master, given that the PR worked fine? And why your master is producing a different result?

vsoch commented 4 years ago

I'll point out that your fix branch has that same message:

Enter host password for user 'jascodes':

JasCodes commented 4 years ago

I know!! but it don't say this after that 😜

Enter host password for user 'jascodes':
{
  "message": "Validation Failed",
  "errors": [
    {
      "resource": "PullRequest",
      "code": "custom",
      "message": "No commits between master and testx"
    }
  ],
  "documentation_url": "https://developer.github.com/v3/pulls/#create-a-pull-request"
}
JasCodes commented 4 years ago

Oh I see why you might getting confused I was sharing my fork of your action without fix... I hope one build before that will clear things up https://github.com/jascodes/ga/runs/435610939?check_suite_focus=true

vsoch commented 4 years ago

hey @jascodes ! The official answer from GitHub is that both Bearer and token should work, and I just tested this on my branch to confirm that the current master still works:

https://github.com/vsoch/pull-request-action-example/pull/2

I also created a bearer branch here that makes that one change, and tested it the same, and the result is the same, down to the messages printed. GitHub also tested and they came to these conclusions:

So basically, we took the example from https://help.github.com/en/actions/configuring-and-managing-workflows/authenticating-with-the-github_token#example-calling-the-rest-api and made a request with the authorization header using Bearer and then another request using token and both requests succeeded.

The only way I was able to reproduce your error:

{
  "message": "Validation Failed",
  "errors": [
    {
      "resource": "PullRequest",
      "code": "custom",
      "message": "A pull request already exists for jascodes:textx2."
    }
  ],
  "documentation_url": "https://developer.github.com/v3/pulls/#create-a-pull-request"
}

was by (using either Bearer or token) opening a pull request for a branch, and then trying to do it again - you will see that failure message when the request does not succeed. The pattern is consistent with what I see here - successes followed by failures, and I suspect this is because the branch was open for PR:

image

So, I don't have enough evidence to make this change, both result in the same behavior, and I believe the error to be an artifact that I've shown above. I greatly appreciate your thoughtful contribution and I'm happy to leave this open in case things change.

vsoch commented 4 years ago

And please note the specific messages you've received on failure - they are not due to the token:

And I haven't looked beyond what we've shared here, but there could be others. So indeed there is an error, just not related to the authentication header use of token or Bearer.

vsoch commented 4 years ago

@jascodes but one thing you might be interested in testing is if the return value of the "does not work" set of messages can better indicate failure, and then we could stop the workflow there. What do you think?

JasCodes commented 4 years ago

I am going to review.. and get back to you; going to be busy week.

Thx

vsoch commented 4 years ago

No worries! I'm always around, ping me when you'd like to continue discussion.