Skip to content

[PECO-1687] Fixed the issue of infinite blocking in case of invalid credentials #399

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 6 commits into from
Closed

[PECO-1687] Fixed the issue of infinite blocking in case of invalid credentials #399

wants to merge 6 commits into from
+5 −2

Conversation

jprakash-db
Copy link
Contributor

@jprakash-db jprakash-db commented Jun 17, 2024 •
edited
Loading

  • Added the error handling for the Invalid Credentials error, that caused the connection retry to be stuck in an infinite loop

@jprakash-db jprakash-db self-assigned this Jun 17, 2024
kravets-levko
kravets-levko suggested changes Jun 17, 2024
View reviewed changes
Copy link
Contributor

@kravets-levko kravets-levko left a comment

Choose a reason for hiding this comment

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

Hi @jprakash-db! Thank you for your effort on fixing this issue! Changes look good, but please do few things before we can merge this:

  • remove .idea folder
  • use existing NonRecoverableNetworkError instead of AuthenticationFailureError

@jprakash-db
Copy link
Contributor Author

@kravets-levko Sure, I will make those changes

@kravets-levko
Copy link
Contributor

@jprakash-db please also remove the .idea folder from this PR

@jprakash-db
Copy link
Contributor Author

@jprakash-db please also remove the .idea folder from this PR

Removed it @kravets-levko

@kravets-levko
Copy link
Contributor

@jprakash-db Can you please resolve conflicts, so I can merge this PR? Thanks!

@jprakash-db
Copy link
Contributor Author

@kravets-levko Resolved the merge conflicts

@kravets-levko
Copy link
Contributor

@jprakash-db when you resolved conflicts, I got confused, because I clearly remember that your PR definitely contained a code for handing 401 status. And then, I realized that recently we merged #408 🤦‍♂️ Sorry for asking you to do the job which is no longer needed. Fortunately, such overlaps don't happen often

@jprakash-db
Copy link
Contributor Author

@kravets-levko Ya, while merging I too saw, that someone had already fixed it 😅

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

@gopalldb gopalldb Awaiting requested review from gopalldb

@samikshya-db samikshya-db Awaiting requested review from samikshya-db samikshya-db is a code owner

@jayantsing-db jayantsing-db Awaiting requested review from jayantsing-db

@rcypher-databricks rcypher-databricks Awaiting requested review from rcypher-databricks

@yunbodeng-db yunbodeng-db Awaiting requested review from yunbodeng-db yunbodeng-db is a code owner

@andrefurlan-db andrefurlan-db Awaiting requested review from andrefurlan-db

@jackyhu-db jackyhu-db Awaiting requested review from jackyhu-db jackyhu-db is a code owner

@benc-db benc-db Awaiting requested review from benc-db benc-db is a code owner

@kravets-levko kravets-levko Awaiting requested review from kravets-levko

Assignees

@jprakash-db jprakash-db

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jprakash-db @kravets-levko