Queue.Item.authenticate honors QueueItemAuthenticatorProvider; Tasks.getAuthenticationOf should as well #2880

Merged
merged 2 commits into from May 19, 2017

Conversation

4 participants
@jglick
Member

jglick commented May 9, 2017

Description

Completes 65b71c2.

Changelog entries

  • RFE: Internal API: Tasks.getAuthenticationOf now honors authentication contributed by QueueItemAuthenticatorProvider extensions

Desired reviewers

@reviewbybees

@jglick jglick requested a review from stephenc May 9, 2017

@reviewbybees

This comment has been minimized.

Show comment
Hide comment
@reviewbybees

reviewbybees May 9, 2017

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation.

@oleg-nenashev

oleg-nenashev approved these changes May 12, 2017 edited

🐝 . Maybe the original getAuthenticators() method should be restricted and documented

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
Member

oleg-nenashev commented May 12, 2017

@jglick

This comment has been minimized.

Show comment
Hide comment
@jglick

jglick May 12, 2017

Member

Maybe the original getAuthenticators() method should be restricted

I see no need for that; it does what it claims. But I agree it would be wise to update its documentation.

Member

jglick commented May 12, 2017

Maybe the original getAuthenticators() method should be restricted

I see no need for that; it does what it claims. But I agree it would be wise to update its documentation.

@oleg-nenashev

re-🐝

@daniel-beck

This comment has been minimized.

Show comment
Hide comment
@daniel-beck

daniel-beck May 15, 2017

Member

no known user impact

Are there no implementations that suddenly take on new authentications?

Won't this enable new use cases for plugin developers?

Member

daniel-beck commented May 15, 2017

no known user impact

Are there no implementations that suddenly take on new authentications?

Won't this enable new use cases for plugin developers?

@oleg-nenashev

This comment has been minimized.

Show comment
Hide comment
@oleg-nenashev

oleg-nenashev May 16, 2017

Member

Yeah, I would rather note it as API/Improvement entry at least. Does not hurt

Member

oleg-nenashev commented May 16, 2017

Yeah, I would rather note it as API/Improvement entry at least. Does not hurt

@jglick

This comment has been minimized.

Show comment
Hide comment
@jglick

jglick May 16, 2017

Member

Are there no implementations

None open source that I am aware of. There is an implementation in a CloudBees product.

Member

jglick commented May 16, 2017

Are there no implementations

None open source that I am aware of. There is an implementation in a CloudBees product.

@oleg-nenashev oleg-nenashev requested a review from daniel-beck May 17, 2017

@oleg-nenashev oleg-nenashev merged commit 8199ec5 into jenkinsci:master May 19, 2017

1 check passed

continuous-integration/jenkins/pr-head This commit looks good
Details

@jglick jglick deleted the jglick:Tasks.getAuthenticationOf-QueueItemAuthenticatorProvider branch May 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment