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

[feat][python] Add basic authentication #17482

Merged
merged 2 commits into from
Sep 7, 2022

Conversation

nodece
Copy link
Member

@nodece nodece commented Sep 6, 2022

Motivation

Python misses the basic authentication.

Verifying this change

Added test_basic_auth test to cover this feature.

Documentation

Check the box below or label this PR directly.

Need to update docs?

  • doc-required
    Add Python basic authentication documentation.

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@github-actions github-actions bot added the doc-required Your PR changes impact docs and you will update later. label Sep 6, 2022
Copy link
Contributor

@BewareMyPower BewareMyPower left a comment

Choose a reason for hiding this comment

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

Could you add tests for a wrong username or secret? It's better to show what will happen in this case.

@codelipenghui
Copy link
Contributor

#15822 has merged to branch-2.11. It's good to have this one in 2.11.0

Signed-off-by: Zixuan Liu <[email protected]>
@nodece nodece requested a review from BewareMyPower September 6, 2022 10:06
@nodece
Copy link
Member Author

nodece commented Sep 6, 2022

Could you add tests for a wrong username or secret? It's better to show what will happen in this case.

I added this test, please help review, thanks!

@codelipenghui codelipenghui merged commit eae90f6 into apache:master Sep 7, 2022
codelipenghui pushed a commit that referenced this pull request Sep 7, 2022
momo-jun added a commit to momo-jun/pulsar that referenced this pull request Sep 14, 2022
@momo-jun momo-jun removed the doc-required Your PR changes impact docs and you will update later. label Sep 14, 2022
@momo-jun momo-jun added the doc-complete Your PR changes impact docs and the related docs have been already added. label Sep 14, 2022
momo-jun added a commit that referenced this pull request Sep 20, 2022
…hapter (#17615)

* Sidebar re-org

* Separate TLS encryption/authentication using Keystore to parent topics

* use language-specific tabs to show code snippets

* streamline headings

* Update client-libraries-java.md

* Add code snippet for python clients to support #17482

* Add code snippet for go clients
BewareMyPower added a commit to BewareMyPower/pulsar-client-python that referenced this pull request Oct 13, 2022
### Motivation

apache/pulsar#17482 supported basic
authentication for Python client, but the `AuthenticationBasic` class
accepts two positional arguments as the username and password. It's not
good for extension. We should accept an auth param string like
`AuthenticationOauth2` so that no changes are needed if the upstream C++
client's implementation changed, like
apache/pulsar-client-cpp#37.

### Modifications

To be compatible with the existing API, change the first two arguments
to keyword arguments. Then, add the 3rd keyword argument to represent
the auth param string.

### Verifications

`test_basic_auth` and `test_invalid_basic_auth` are extended for this
change.
merlimat pushed a commit to apache/pulsar-client-python that referenced this pull request Oct 13, 2022
* Support auth param string for Basic authentication

### Motivation

apache/pulsar#17482 supported basic
authentication for Python client, but the `AuthenticationBasic` class
accepts two positional arguments as the username and password. It's not
good for extension. We should accept an auth param string like
`AuthenticationOauth2` so that no changes are needed if the upstream C++
client's implementation changed, like
apache/pulsar-client-cpp#37.

### Modifications

To be compatible with the existing API, change the first two arguments
to keyword arguments. Then, add the 3rd keyword argument to represent
the auth param string.

### Verifications

`test_basic_auth` and `test_invalid_basic_auth` are extended for this
change.

* Add method argument and related tests

* Add type check for method arg
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc-complete Your PR changes impact docs and the related docs have been already added.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants