You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2022/10/13 11:09:10 UTC

[GitHub] [pulsar-client-python] BewareMyPower opened a new pull request, #15: Support auth param string for Basic authentication

BewareMyPower opened a new pull request, #15:
URL: https://github.com/apache/pulsar-client-python/pull/15

   ### Motivation
   
   https://github.com/apache/pulsar/pull/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
   https://github.com/apache/pulsar-client-cpp/pull/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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar-client-python] merlimat merged pull request #15: Support auth param string for Basic authentication

Posted by GitBox <gi...@apache.org>.
merlimat merged PR #15:
URL: https://github.com/apache/pulsar-client-python/pull/15


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar-client-python] merlimat commented on a diff in pull request #15: Support auth param string for Basic authentication

Posted by GitBox <gi...@apache.org>.
merlimat commented on code in PR #15:
URL: https://github.com/apache/pulsar-client-python/pull/15#discussion_r994635233


##########
pulsar/__init__.py:
##########
@@ -347,18 +347,32 @@ class AuthenticationBasic(Authentication):
     """
     Basic Authentication implementation
     """
-    def __init__(self, username, password):
+    def __init__(self, username=None, password=None, auth_params_string=None):

Review Comment:
   We should also accept the `method` argument here, otherwise it might not be obvious that you can pass it in the json string



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar-client-python] nodece commented on a diff in pull request #15: Support auth param string for Basic authentication

Posted by GitBox <gi...@apache.org>.
nodece commented on code in PR #15:
URL: https://github.com/apache/pulsar-client-python/pull/15#discussion_r994739602


##########
pulsar/__init__.py:
##########
@@ -347,18 +347,32 @@ class AuthenticationBasic(Authentication):
     """
     Basic Authentication implementation
     """
-    def __init__(self, username, password):
+    def __init__(self, username=None, password=None, auth_params_string=None):

Review Comment:
   I agreed.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org

[GitHub] [pulsar-client-python] BewareMyPower commented on a diff in pull request #15: Support auth param string for Basic authentication

Posted by GitBox <gi...@apache.org>.
BewareMyPower commented on code in PR #15:
URL: https://github.com/apache/pulsar-client-python/pull/15#discussion_r994861895


##########
pulsar/__init__.py:
##########
@@ -347,18 +347,32 @@ class AuthenticationBasic(Authentication):
     """
     Basic Authentication implementation
     """
-    def __init__(self, username, password):
+    def __init__(self, username=None, password=None, auth_params_string=None):

Review Comment:
   Done. PTAL again @merlimat @nodece 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org