Skip to content

Connection check fails on insufficient privileges#18

Merged
GabyUnalaq merged 3 commits intomainfrom
16_connection_check
Jun 10, 2025
Merged

Connection check fails on insufficient privileges#18
GabyUnalaq merged 3 commits intomainfrom
16_connection_check

Conversation

@HorjuRares
Copy link
Contributor

@HorjuRares HorjuRares commented Jun 6, 2025

Issues: #16

Definition of Done

The PR shall be merged only if all items mentioned in CONTRIBUTING.md have been followed. In case an item is not applicable as described, please provide a short explanation in the description.

@HorjuRares HorjuRares requested a review from GabyUnalaq June 6, 2025 12:01
@HorjuRares HorjuRares self-assigned this Jun 6, 2025
@HorjuRares HorjuRares added ready for review The issue is ready for review and removed ready for review The issue is ready for review labels Jun 6, 2025
@HorjuRares HorjuRares added the ready for review The issue is ready for review label Jun 6, 2025
Copy link
Contributor

@GabyUnalaq GabyUnalaq left a comment

Choose a reason for hiding this comment

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

There are 2 cases for when the check if successful:

  • The CompleteState with only the apiVersion is returned, thus the connection is fine.
  • The request is blocked due to not enough privileges, thus the connection is fine.

In all other cases (ConnectionClosed or ProtocolException), the connection check fails. Because the connection_established variable never gets set to True if the apiVersion is successfully returned, the error line in get state for subsequent requests never get's logged in case of a privilege issue.

Copy link
Contributor

@GabyUnalaq GabyUnalaq left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@GabyUnalaq GabyUnalaq merged commit 6cbff39 into main Jun 10, 2025
9 checks passed
@GabyUnalaq GabyUnalaq deleted the 16_connection_check branch June 10, 2025 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready for review The issue is ready for review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants