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

Consistent connection key #19

Closed
basz opened this issue Mar 25, 2018 · 6 comments
Closed

Consistent connection key #19

basz opened this issue Mar 25, 2018 · 6 comments

Comments

@basz
Copy link
Member

basz commented Mar 25, 2018

A minor inconsistency; Throughout the various components the connection configuration key for persistency connections. Only in the snapshot packages 'connection_service' is the mandatory key.

https://github.com/search?q=org%3Aprooph+connection_service&type=Code

Dunno how to fix this without a BC...

@prolic
Copy link
Member

prolic commented Mar 25, 2018 via email

@basz
Copy link
Member Author

basz commented Mar 25, 2018 via email

@prolic
Copy link
Member

prolic commented Mar 25, 2018

@sandrokeil any ideas?

@sandrokeil
Copy link
Member

I don't think we can solve this with interop config. We need something like an alias but I don't want to add such a feature. This is not really an issue but we can fix it in next major versions.

Or do you want to introduce connection and keep connection_service key? We could remove connection_service from mandatory list and update docs to use connection. Then it would fail with an service not found exception if not configured properly. The factory could take care to check which key should be used.

@basz
Copy link
Member Author

basz commented Mar 26, 2018

what if we;

  1. remove the connection_sercvice from mandatory option
  2. throw MandatoryOptionNotFoundException::missingOption($this->dimensions(), 'connection' if neither connection or connection_service is found in __invoke.
  3. Oh, and update docs

Behaviour would be equal to what would happen now except for the reported key in the exception.

@sandrokeil
Copy link
Member

Good idea, I think we can do this manually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants