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

DEV-266 - Return null for discovery mode or multiple hosts in connection string #280

Conversation

w1am
Copy link
Contributor

@w1am w1am commented Jan 31, 2024

Changed: Return null for discovery mode or multiple hosts in connection string
Changed: Updated Address field to support null returns
Fixed: #260

This PR refactors the Address property resolution logic in the EventStoreClientConnectivitySettings class. Previously, when a cluster was specified in the connection string, the Address property defaulted to localhost:2113, which was inconsistent behavior. With this change, the logic now returns null when a cluster is specified. New tests have also been added to cover various connection string scenarios.

Changes

Connection string Behaviour before Behaviour after
esdb://localhost:1234 localhost:1234 Unchanged
esdb://localhost:1234,localhost:4567 localhost:2113 null
esdb+discover://localhost:1234 localhost:2113 null
esdb+discover://localhost:1234 localhost:2113 null
esdb+discover://localhost:1234,localhost:4567 localhost:2113 null

Copy link

linear bot commented Jan 31, 2024

@w1am w1am marked this pull request as draft February 2, 2024 04:48
@w1am w1am force-pushed the w1am/dev-266-improve-connectivity-settings-address-resolution-logic branch from 0d0f068 to 0ef472f Compare February 6, 2024 06:03
@w1am w1am changed the title Refactor address resolution logic in connectivity settings Return null for single host in discovery mode Feb 6, 2024
@w1am w1am marked this pull request as ready for review February 6, 2024 06:15
@w1am w1am changed the title Return null for single host in discovery mode Return null for discovery mode or multiple hosts in connection string Feb 6, 2024
@w1am w1am closed this Feb 6, 2024
@w1am w1am force-pushed the w1am/dev-266-improve-connectivity-settings-address-resolution-logic branch from 0ef472f to def981a Compare February 6, 2024 09:30
josephcummings
josephcummings previously approved these changes Feb 6, 2024
Copy link
Contributor

@josephcummings josephcummings left a comment

Choose a reason for hiding this comment

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

👍

@josephcummings josephcummings changed the title Return null for discovery mode or multiple hosts in connection string DEV-266 - Return null for discovery mode or multiple hosts in connection string Feb 26, 2024
@josephcummings josephcummings merged commit a0c283b into master Feb 26, 2024
60 checks passed
@josephcummings josephcummings deleted the w1am/dev-266-improve-connectivity-settings-address-resolution-logic branch February 26, 2024 13:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connection string port is parsed wrong
3 participants