Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

[Messenger] [Redis] Makeauth option works#42067

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
chalasr merged 1 commit intosymfony:5.2fromwelcoMattic:patch-1
Jul 13, 2021

Conversation

@welcoMattic
Copy link
Member

@welcoMatticwelcoMattic commentedJul 12, 2021•
edited
Loading

QA
Branch?5.3
Bug fix?yes
New feature?no
Deprecations?no
Tickets
LicenseMIT
Doc PR

Considering this configuration:

framework:messenger:transports:queue:dsn:'%env(MESSENGER_TRANSPORT_DSN)%'options:stream:'cms'delete_after_ack:falseauth:'pa$$word'serializer:!php/const Redis::SERIALIZER_JSON

It results to this error:

In Connection.php line 510:  NOAUTH Authentication required.

Because theauth option was never read from the options, only from the parsed DSN.

This fix allows users to useauth option from the configuration.

I target 5.3, as 5.2 is going to be unmaintained at the end of July

@chalasr
Copy link
Member

A test case would be nice to prevent regressions

welcoMattic reacted with thumbs up emoji

@welcoMattic
Copy link
MemberAuthor

welcoMattic commentedJul 12, 2021•
edited
Loading

@chalasr Sure, I'm on it. I will push it later today

EDIT: sorry for the messy rebase on 5.3

@welcoMatticwelcoMattic changed the base branch from5.4 to5.2July 13, 2021 04:52
@welcoMatticwelcoMattic changed the base branch from5.2 to5.4July 13, 2021 04:53
@welcoMatticwelcoMattic changed the base branch from5.4 to5.3July 13, 2021 04:57
@welcoMatticwelcoMattic changed the base branch from5.3 to5.4July 13, 2021 04:57
@welcoMatticwelcoMattic changed the base branch from5.4 to5.3July 13, 2021 05:00
@welcoMattic
Copy link
MemberAuthor

welcoMattic commentedJul 13, 2021•
edited
Loading

As documentation mention "Options defined under options take precedence over ones defined in the DSN." (https://symfony.com/doc/current/messenger.html#transport-configuration), I've updated the code to respect that. Also, I've added test to assert that put the auth value at user or password place in the DSN doesn't matter.

dunglas reacted with thumbs up emoji

@chalasrchalasr added this to the5.2 milestoneJul 13, 2021
@chalasrchalasr changed the base branch from5.3 to5.2July 13, 2021 07:24
@chalasr
Copy link
Member

Good catch, thanks@welcoMattic.

@chalasrchalasr merged commit6c26499 intosymfony:5.2Jul 13, 2021
@chalasr
Copy link
Member

Merged on 5.2 as this is a quick win and we didn't discuss about freezing 5.2 yet.

welcoMattic reacted with thumbs up emoji

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@dunglasdunglasdunglas approved these changes

@chalasrchalasrchalasr approved these changes

Assignees

No one assigned

Projects

None yet

Milestone

5.2

Development

Successfully merging this pull request may close these issues.

4 participants

@welcoMattic@chalasr@dunglas@carsonbot
[8]ページ先頭
©2009-2025 Movatter.jp