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

fix(routing): updating mediator pickupStrategy #809

Closed
wants to merge 1 commit into from
Closed

fix(routing): updating mediator pickupStrategy #809

wants to merge 1 commit into from

Conversation

Iskander508
Copy link
Contributor

Fixed bug with updating mediator pickupStrategy

Fixes #807

@TimoGlastra
Copy link
Contributor

Your approach is cleaner than my approach so inclined to merge this one. However it does have some impact when you change configurations.

In your approach the following happens:

  1. Agent config specifies mediator pickup strategy
  2. agent config value is stored in mediator record
  3. Next time you use it the mediator record value will be used, regardless whether the agent config value is changed

in my approach the following happens:

  1. Agent config specifies mediator pickup strategy
  2. agent config value is not stored in mediator record
  3. Next time you use it the agent config value will be used -> this matters when you change the agent config value

Any thoughts on which of those would be preferred here? For others protocols we normally use the second appraoch (so not storing a global config in a specific record). If I start the agent with autoAcceptConnections enabled, it won't actually store that in the connectino record. If I restart the agent next time with autoAcceptConnections false it will take it from the agent config, and doesn't have stored the autoAcceptConnection true on the connection record

@Iskander508 Iskander508 changed the title Fixed updating mediator pickupStrategy fix(routing): updating mediator pickupStrategy May 27, 2022
@Iskander508
Copy link
Contributor Author

Any thoughts on which of those would be preferred here?

No preference TBH, both should fix our case. If there's some pattern used in other places, it might be better to follow it, keeping better consistency and readability.

@TimoGlastra
Copy link
Contributor

I merged #808 to keep it consistent with the rest of the framework. Could you check with the latest alpha if your problem has been resolved?

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.

[alpha.88+] Websocket connection to mediator not working
2 participants