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

All uris must be reachable in case of environment creation #309

Closed
Egyptmaster opened this issue Jun 7, 2024 · 6 comments · Fixed by #310
Closed

All uris must be reachable in case of environment creation #309

Egyptmaster opened this issue Jun 7, 2024 · 6 comments · Fixed by #310
Labels
bug Something isn't working

Comments

@Egyptmaster
Copy link

Describe the bug

Not sure if that is an expected behavior but it makes the usage of the client very hard. In case you are using multiple uris by SetUris(uris ...string) method all the hosts must be available at the point in time you create the environment with NewEnvironment.

Reproduction steps

env, err = stream.NewEnvironment(
		stream.NewEnvironmentOptions().
			SetUris([]string{"rabbitmq-streams://localhost:5552", "rabbitmq-streams://localhost:5553"}))

will return an error in case one of the uris is not accessable at point in time

Expected behavior

From the documentation I would expect to silently accept one of the hosts/uris to be not available at any point in time and automatically use randomly another one. Otherwise it is really hard in context of kubernetes/docker when I can never guarantee that all nodes are available when my service tries to start.

Additional context

No response

@Egyptmaster Egyptmaster added the bug Something isn't working label Jun 7, 2024
@Gsantomaggio
Copy link
Member

Thank you @Egyptmaster

yes, his part can be improved:

for _, parameter := range options.ConnectionParameters {
if parameter.Uri != "" {
u, err := url.Parse(parameter.Uri)
if err != nil {
return nil, err
}
parameter.Scheme = u.Scheme
parameter.User = u.User.Username()
parameter.Password, _ = u.User.Password()
parameter.Host = u.Host
parameter.Port = u.Port()
if vhost := strings.TrimPrefix(u.Path, "/"); len(vhost) > 0 {
if vhost != "/" && strings.Contains(vhost, "/") {
return nil, errors.New("multiple segments in URI path: " + u.Path)
}
parameter.Vhost = vhost
}
}
parameter.mergeWithDefault()
client.broker = parameter
}
return &Environment{

The client should try to connect with each URI

Gsantomaggio added a commit that referenced this issue Jun 7, 2024
Fixes: #309
Signed-off-by: Gabriele Santomaggio <g.santomaggio@gmail.com>
Gsantomaggio added a commit that referenced this issue Jun 7, 2024
Fixes: #309
Signed-off-by: Gabriele Santomaggio <g.santomaggio@gmail.com>
Gsantomaggio added a commit that referenced this issue Jun 7, 2024
Fixes: #309
Signed-off-by: Gabriele Santomaggio <g.santomaggio@gmail.com>
@Gsantomaggio
Copy link
Member

@Egyptmaster can you please try #310 ? thank you

@Egyptmaster
Copy link
Author

Sure, but can do on Monday earliest

@Gsantomaggio
Copy link
Member

@Egyptmaster, please let me know if you have a chance to test it so I can release the new version. Thank you

@Egyptmaster
Copy link
Author

@Gsantomaggio sorry for late response. I recently tried the feature brach and the issue seems to be fixed 👍
thanks for the quick fix

Gsantomaggio added a commit that referenced this issue Jun 12, 2024
* Fix multi uris connection
* Fixes: #309


---------
Signed-off-by: Gabriele Santomaggio <g.santomaggio@gmail.com>
@Gsantomaggio
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants