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

Allow to pass DSN to the Elastica\Client constructor #1640

Closed
renanbr opened this issue Jun 14, 2019 · 3 comments

Comments

@renanbr
Copy link

@renanbr renanbr commented Jun 14, 2019

Currently I have to handle config options as separated keys in each environment my application is deployed (local, ci, staging, prod...). Things become hard when an environment required an option the other don't...

It could be cool if Elastica\Client's constructor becomes able to parse a single string.

new Elastica\Client('192.168.1.1:9200');
new Elastica\Client('https://user:p4ss@foo.com:9200/elastic;persistent=false;retryOnConflict=2');
@tacman

This comment has been minimized.

Copy link

@tacman tacman commented Jul 16, 2019

Yes, this would be particular helpful on deployments like Heroku, which creates the DSN environment variable when the service is created, but configuring it requires manually breaking it apart into the individual components.

@ruflin

This comment has been minimized.

Copy link
Owner

@ruflin ruflin commented Aug 12, 2019

Do you see a way how we could introduce this without making it a breaking change? In general, +1 on having such a feature.

alamirault pushed a commit to alamirault/Elastica that referenced this issue Aug 17, 2019
alamirault pushed a commit to alamirault/Elastica that referenced this issue Aug 25, 2019
alamirault pushed a commit to alamirault/Elastica that referenced this issue Aug 25, 2019
ruflin added a commit that referenced this issue Aug 28, 2019
This PR allow to pass DSN to the Elastica\Client constructor as discussed in #1640 .

For example: $client = new Client('https://user:p4ss@foo.com:9200?persistent=false&retryOnConflict=2');
@alamirault

This comment has been minimized.

Copy link
Contributor

@alamirault alamirault commented Sep 23, 2019

PR is now merge, I think @ruflin you can close issue.

@ruflin ruflin closed this Sep 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.