Fix follower error handling when leader returns invalid compatibility… #1156
+0
−0
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@harishva23 thanks for the pr. Can you add any unit/integration tests ? |
Hi, Thanks for the reply, it will take some time for me to add some tests. Can I add it in this PR itself? |
Yes pls |
harishva23
force-pushed
the
fix/follower-error-handling-clean
branch
from
44025bf to
0279ec3
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
About this change - What it does
This PR fixes 2 issues - error handling inside the follower and error handling for file-based basic authentication
Earlier, the follower could not parse error messages from leader if write requests were made to the follower and it failed due to some error like incompatible or invalid schema , trying to delete a non-existent schema etc. Now this is able to handle these error messages and pass it on to the user or client.
In the file-based authentication, if a non-existent user was sent in the Headers ( a username not in auth file), karapace threw an Internal Server Error instead of Unauthorized message. This did not happen when a correct username but wrong password was passed. So I have fixed this issue and it returns an Unauthorized error message for both the cases.
References: #1154
Why this way
Since Confluent client always expects a FastAPI response, I have changed the code to return proper format of error messages from follower. This leads to consistent error messages from both leader and follower so that the user can decide the next steps.
For the file-based authentication, raising a generic ValueError results in Internal Server Error being thrown and a full stacktrace appears in the logs which reveals that User is not found. Since karapace already throws a Unauthorised Error if wrong password is sent, I have removed the logic raising ValueError and instead returning None type which is then handled in the auth.py file in the authenticate function.