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

Add a default timeout for OAuth2 Metadata Resolver #14056

Merged
merged 1 commit into from
Jan 30, 2022

Conversation

addisonj
Copy link
Contributor

Currently, there is no timeout on the metadata resolver in oauth2
clients.

This can result in a connection hanging indefintely.

We should add sane defaults here.

These defaults match the defaults of the TokenClient and generally the
Pulsar defaults of 10s connect timeout and 30s read timeout

Test

This seems to primarily depend on testing timeout of the java HTTP lib, so no test is likely needed.

Doc

This is a trivial change and doesn't require docs

Currently, there is no timeout on the metadata resolver in oauth2
clients.

This can result in a connection hanging indefintely.

We should add sane defaults here.

These defaults match the defaults of the TokenClient and generally the
Pulsar defaults of 10s connect timeout and 30s read timeout
@github-actions
Copy link

@addisonj:Thanks for your contribution. For this PR, do we need to update docs?
(The PR template contains info about doc, which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? Thanks)

@codelipenghui codelipenghui added the type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages label Jan 30, 2022
@codelipenghui codelipenghui added this to the 2.10.0 milestone Jan 30, 2022
Copy link
Contributor

@hangc0276 hangc0276 left a comment

Choose a reason for hiding this comment

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

LGTM

@hangc0276 hangc0276 merged commit d111476 into apache:master Jan 30, 2022
@codelipenghui codelipenghui deleted the oauth2_resolve_timeout branch January 30, 2022 09:02
merlimat pushed a commit that referenced this pull request Feb 1, 2022
## Motivation
Currently, there is no timeout on the metadata resolver in oauth2
clients.

This can result in a connection hanging indefintely.

We should add sane defaults here.

## Modification
These defaults match the defaults of the TokenClient and generally the
Pulsar defaults of 10s connect timeout and 30s read timeout

## Test

This seems to primarily depend on testing timeout of the java HTTP lib, so no test is likely needed.

## Doc

This is a trivial change and doesn't require docs
merlimat pushed a commit that referenced this pull request Feb 1, 2022
## Motivation
Currently, there is no timeout on the metadata resolver in oauth2
clients.

This can result in a connection hanging indefintely.

We should add sane defaults here.

## Modification
These defaults match the defaults of the TokenClient and generally the
Pulsar defaults of 10s connect timeout and 30s read timeout

## Test

This seems to primarily depend on testing timeout of the java HTTP lib, so no test is likely needed.

## Doc

This is a trivial change and doesn't require docs
@merlimat merlimat added cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life labels Feb 1, 2022
Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022
## Motivation
Currently, there is no timeout on the metadata resolver in oauth2
clients.

This can result in a connection hanging indefintely.

We should add sane defaults here.

## Modification
These defaults match the defaults of the TokenClient and generally the
Pulsar defaults of 10s connect timeout and 30s read timeout

## Test

This seems to primarily depend on testing timeout of the java HTTP lib, so no test is likely needed.

## Doc

This is a trivial change and doesn't require docs
@Anonymitaet Anonymitaet added doc-not-needed Your PR changes do not impact docs and removed doc-label-missing labels Apr 25, 2022
@Anonymitaet
Copy link
Member

When submitting a PR, can you provide doc-related info (tick the box) in the PR description? So that Bot can recognize and label your PR correspondingly. Or else Bot labels your PR with doc-info-missing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/client cherry-picked/branch-2.8 Archived: 2.8 is end of life cherry-picked/branch-2.9 Archived: 2.9 is end of life doc-not-needed Your PR changes do not impact docs release/2.8.3 release/2.9.2 type/enhancement The enhancements for the existing features or docs. e.g. reduce memory usage of the delayed messages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants