Skip to content

[Sqs] Allow array-based DSN configuration #315

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

Merged
merged 6 commits into from
Jan 13, 2018

Conversation

beryllium
Copy link
Contributor

I encountered a problem with using DSN-based configuration in Symfony 4, where the 'dsn' value was always being passed as an array key. The constructor did not properly handle this, and the primary symptom of this was a constant complaint that the 'region' was not set, even though it was present in the DSN.

I encountered a problem with using DSN-based configuration in Symfony 4, where the 'dsn' value was always being passed as an array key. The constructor did not properly handle this, and the primary symptom of this was a constant complaint that the 'region' was not set, even though it was present in the DSN.
throw new \LogicException('The config must be either an array of options, a DSN string, or null');
}

if (count($config) == 1 && array_key_exists('dsn', $config)) {
Copy link
Member

Choose a reason for hiding this comment

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

dsn option could be used with other options in config. It should work like here

The config is treat as default options, and options from dsn could overwrite them

@makasim makasim added the bug label Jan 9, 2018
@beryllium
Copy link
Contributor Author

Looks like the tests are failing due to a misconfiguration with IAM credentials? Unless I'm mistaken, it doesn't seem to be related to the PR.

@makasim
Copy link
Member

makasim commented Jan 12, 2018

It is a security policy, Travis does not add secret envs vars if PR author is not member of org.

I'll check it myself and merge.

@makasim
Copy link
Member

makasim commented Jan 12, 2018

that should fix it dfd4499

@beryllium
Copy link
Contributor Author

Merged the latest master, hopefully everything will pass now :)

@makasim makasim merged commit 826b41b into php-enqueue:master Jan 13, 2018
@makasim
Copy link
Member

makasim commented Jan 13, 2018

Thank you

ASKozienko pushed a commit that referenced this pull request Nov 2, 2018
[Sqs] Allow array-based DSN configuration
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants